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

feat: split GenomeAuthoritySystem based on modules #120

Merged
merged 3 commits into from
Aug 16, 2021

Conversation

skaldarnar
Copy link
Contributor

@skaldarnar skaldarnar commented Aug 13, 2021

The integration with Genome and BasicCrafting is optional for SimpleFarming.

As a component system can only be loaded properly if all it's dependencies are also active, this means that the combined GenomeAuthoritySystem is only loaded if both Genome and BasicCrafting are active.

By splitting the authority system into GenomeExtensionAuthoritySystem and GenomeCraftingExtensionAuthoritySystem each extension system can run with the minimal set of required modules.

  • GenomeExtensionAuthoritySystem (works if at least Genome is loaded)
  • GenomeCraftingExtensionAuthoritySystem (works if both Genome and Basic Crafting are loaded)

Fixes #119
Closes #103

The integration with Genome and BasicCrafting is optional for SimpleFarming.

As a component system can only be loaded properly if all it's dependencies are also active, this means that the combined `GenomeAuthoritySystem` is only loaded if **both** Genome and BasicCrafting are active.

By splitting the authority system into `GenomeExtensionAuthoritySystem` and `GenomeCraftingExtensionAuthoritySystem` each extension system can run with the minimal set of required modules.

- `GenomeExtensionAuthoritySystem` (works if at least Genome is loaded)
- `GenomeCraftingExtensionAuthoritySystem` (works if both Genome and Basic Crafting are loaded)

Fixes #119
@skaldarnar skaldarnar added Type: Improvement Request for or addition/enhancement of a feature Type: Refactoring Request for or implementation of pure and automatic refactorings, e.g. renaming, to improve clarity labels Aug 13, 2021
@keturn
Copy link
Contributor

keturn commented Aug 13, 2021

Does this also fix #118?

@skaldarnar
Copy link
Contributor Author

Doesn't look like the tests are fixed: see pipeline

…systems

I don't really know what the effect of that is, but it kind of makes sense to use the annotation field to document the optional dependencies in a way that we can more easily pick them up when registering systems.
@skaldarnar skaldarnar merged commit 636c7a2 into develop Aug 16, 2021
@skaldarnar skaldarnar deleted the feat/split-GenomeAuthoritySystem branch August 16, 2021 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Improvement Request for or addition/enhancement of a feature Type: Refactoring Request for or implementation of pure and automatic refactorings, e.g. renaming, to improve clarity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BasicCrafting and Genome at GenomeAuthoritySystem
3 participants