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

Added Genome related events #93

Merged
merged 8 commits into from
Aug 11, 2020
Merged

Conversation

vedant-shroff
Copy link
Contributor

@vedant-shroff vedant-shroff commented Jun 16, 2020

added genome related events to be handled by the GenomeAuthoritySystem if Genomes is enabled.

To-do before merging

  • Write modifications for non sustainable bushes to inherit GenomeComponent and not throw NPE
  • Major Code Cleanup
  • Remove Genome Dependency. That was only for testing purposes
  • Add JavaDoc in the new Handlers and events

@vedant-shroff vedant-shroff marked this pull request as ready for review June 25, 2020 13:24
Copy link
Member

@syntaxi syntaxi left a comment

Choose a reason for hiding this comment

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

Overall seems fine,
I am a little bit wary of the TransferGenomeEvent
I've put more details about this and more in the review for
Terasology/EdibleFlora#5

module.txt Show resolved Hide resolved
if (!isInLastStage(bushComponent)
// allow negative growth from the last stage
|| stages < 0) {
bush.send(new AddGenomeRetention());
Copy link
Member

Choose a reason for hiding this comment

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

This feels like a bit clunky and weird to be doing through events tbh. That being said, given the state of optional dependencies this possibly is the best way to do this.

newBush.addOrSaveComponent(bushComponent);

if (stage.getValue().maxTime > 0 && stage.getValue().minTime > 0) {
resetDelay(newBush,
stage.getValue().minTime,
stage.getValue().maxTime);
}

bush.send(new TransferGenomeEvent(newBush));
Copy link
Member

Choose a reason for hiding this comment

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

This event by contrast seems a bit weird than that there should be a better way to do it.

@e-aakash e-aakash merged commit 3cd8ac7 into Terasology:develop Aug 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants