WhoCraft / TardisRefined

A Minecraft mod for Forge and Fabric which brings the TARDIS from Doctor Who to Minecraft.
Other
58 stars 23 forks source link

Resolve Registry Tech Debt + Pattern Fixes #182

Closed 50ap5ud5 closed 11 months ago

50ap5ud5 commented 11 months ago

About

This PR significantly cleans up technical debt related to following systems:

Why this is needed

Technical Debt of Theme IDs using Enums

Since the beginning of the mod, the unique identifiers for a Tardis Exterior Shell (ShellTheme) and Console Unit (ConsoleTheme) were using enum which, while suitable for the basic prototype we were creating at the time, would not be able to scale as the mod increases in usage and features.

The mod has reached a point where it has gained significant attention. The design of the Themes using an enum has created following problems:

  1. Any addon mods would not be able to add new Shells or Consoles without the use of Mixins which are difficult to debug and time consuming to maintain.
  2. Bar of entry to create addon content is significantly higher and prone to major mistakes made. New developers who are not experienced with Mixins may develop poor practices that will result in poor quality addon mods being made. We as the developers will also be forced to support them which is significantly extra work
  3. Scalability issues. An enum will not be able to scale when additional content is added. We will need to deal with huge lines of switch statements, hardcoding and other poor practices that will increase code maintenance issues

Pattern Merging

Due to the aforementioned ConsoleTheme and ShellTheme being enums, the resulting ConsolePattern and ShellPattern were restricted to only accounting for Tardis Refined entries. Additionally, the decision to re-design Patterns to become data driven, whilst the Theme entry was still an enum presented multiple problems:

  1. There was no way to merge patterns added by datapacks to an existing Tardis Refined Shell or Console. E.g. Console = tardis_refined:copper, user adds my_awesome_pack:copper_variant_pattern, this would not be possible.
  2. Because patterns were restricted to using ConsoleTheme or ShellTheme entries from Tardis Refined, we needed to hardcode every datapack being parsed, to use the tardis_refined namespace. a. This would make debugging of datapacks confusing because every namespace mentioned in log files would always be tardis_refined i. E.g. Datapack folder is structured as data/my_awesome_pack/tardis_refined/patterns, the parsed entry would be data/tardis_refined/tardis_refined/patterns
  3. Additionally, if users wanted to add patterns to Shells or Consoles added by addons, the hardcoding of the tardis_refined means they cannot do so. This has led to lower adoption figures for Pattern datapacks due to both ourselves and the players not understanding exactly how to format the patterns.

Changes made

Custom Registries and their implementation on Themes

Due to Mojang implementing their own versions of Forge's registries, both NeoForge and Fabric have removed much of their custom registry object overhead and defaulted to using vanilla objects. Following their design patterns this PR has also refactored our registry foundation to use vanilla objects where possible

If multiple datapacks add JSON files with the exact same file name and folder path, the contents of the JSON file will be merged together. Example: 2 datapacks want to add new entries to the Tardis Refined’s “Copper” Console Datapack 1: data/tardis_refined/tardis_refined/patterns/console/copper.json

Datapack 2: data/tardis_refined/tardis_refined/patterns/console/copper.json

Both my_awesome_pack:copper_variant_one and my_other_cool_pack:copper_variant_two will be merged together into a final list of all patterns for files under the data/tardis_refined/tardis_refined/patterns/console/copper.json path. The ID for the list of patterns would be the datapack namespace and the file name i.e. data/tardis_refined/tardis_refined/patterns/console/copper.json -> tardis_refined:copper

Patterns from datapacks parsed later on can now replace entries from datapacks that were parsed earlier

Similar to vanilla Tags, an optional replace Boolean field can be added to the Pattern JSON file to indicate that all entries in the JSON file will overwrite the previous file. Example: 2 datapacks for Tardis Refined’s “Copper” Console, but the second datapack of the datapacks want to replace all entries Datapack 1:

  1. tardis_refined:copper
  2. tardis_refined:copper_sculk

Datapack 2 (Replace set to true)

  1. my_pack:copper_modified
  2. tardis_refined:copper The final list of patterns would be the entries in Datapack 2 as the replace field tells our datapack loader to replace previous entries.
mistersecret312 commented 11 months ago

Ooooo

Jeryn99 commented 11 months ago

Happy to have this merged once conflicts are resolved @50ap5ud5

50ap5ud5 commented 11 months ago

Thanks for fixing the conflicts, I reckon we are good to merge.