Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor package loading, get rid of 'core' non-mod and per-world mod folders #2233

Merged
merged 8 commits into from
Dec 12, 2022

Conversation

olanti-p
Copy link
Contributor

Summary

SUMMARY: Infrastructure "Refactored package loading, got rid of 'core' non-mod and per-world mod folders"

Purpose of change

Streamline package loading, get rid of unused features.

The idea is to transfer from open-ended data loading scheme (initialize loader -> add some packs to it -> either add more packs on top or finalize) to loading from a known set of data packs (pass full list of packs to function -> it initializes loader, adds packs from list, finalizes data).

Along the way, got rid of core non-mod. It's a bit of an outlier, being a non-mod data loading location, and has already caused problems in #2106. Original intent with it was to support total conversion mods (CleverRaven/Cataclysm-DDA#19260), but we still have a lot of definitions in bn pack referred from hardcode, so even if someone were to make a totlal conversion mod for Bright Nights they still would have to copy over parts of bn pack, so existence of core doesn't make any improvement.

Also got rid of per-world mods. It's an obscure feature that's not available (or even indicated) in UI in any way, so almost noone knows about it, and its roots come from some 9 years old spaghetti code, so I'm not sure what its original intended purpose even was (bundling mods with saves? earlier iteration of mod manager, before support for multiple worlds?).

Describe the solution

Cherry-picked from #2216.

Removed all code that used to control DynamicDataLoader's pack loading, and either rewrote it to load data directly (in case of main menu's tips) or encapsulated it in functions in init.h(.cpp).

Testing

Game loads without errors. --check-mods still works.

@github-actions github-actions bot added JSON related to game datas in JSON format. src changes related to source code. tests changes related to tests translation labels Dec 11, 2022
@Coolthulhu Coolthulhu self-assigned this Dec 11, 2022
@Coolthulhu
Copy link
Member

Also got rid of per-world mods. It's an obscure feature that's not available (or even indicated) in UI in any way, so almost noone knows about it, and its roots come from some 9 years old spaghetti code, so I'm not sure what its original intended purpose even was (bundling mods with saves? earlier iteration of mod manager, before support for multiple worlds?).

I actually tried to use it not long ago. Turns out, it's also broken.

@@ -369,11 +369,11 @@ void dependency_tree::clear()
master_node_map.clear();
}

dependency_node *dependency_tree::get_node( mod_id key )
dependency_node *dependency_tree::get_node( mod_id key ) const
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why const if it casts the constness away?
Maybe it should have a const and a non-const variant.
If it's a simple hack to get the job done faster, a comment above it would be nice.

@Coolthulhu Coolthulhu merged commit 708be08 into cataclysmbnteam:upload Dec 12, 2022
@olanti-p olanti-p deleted the refactor-package-loading branch December 17, 2022 22:06
@olanti-p olanti-p mentioned this pull request Apr 12, 2023
27 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JSON related to game datas in JSON format. src changes related to source code. tests changes related to tests translation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants