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

generalize batch event creation #12213

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Susucre
Copy link
Contributor

@Susucre Susucre commented May 2, 2024

The generic method is a little more complex, as the batch event constructor is found and used by reflection.
But the individual addSimultaneousBatch methods are now simpler to comprehend, and new ones will be easier to make.

@github-actions github-actions bot added the engine label May 2, 2024
This should simplify adding new batch events in the future.
@Susucre Susucre force-pushed the genericize-add-batch-event branch from 699659e to 0d3fcb3 Compare May 2, 2024 08:41
@@ -42,6 +42,8 @@
import org.apache.log4j.Logger;

import java.io.Serializable;
import java.lang.reflect.Constructor;
Copy link
Member

Choose a reason for hiding this comment

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

Do not use reflect code in game engine (outside of tests and plugins/class loaders). Use interfaces and basic classes instead (GameEvent classes allow to do it).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well you have to explain how to do that.
I can't make an abstract static method in an interface and/or abstract class that would get Overriden by the inheritors.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that it is best to avoid reflection if possible, though I will note that Watcher class (core game engine) does use reflection already.

(Haven't yet thought about how to make this work with parameterized types without reflection.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants