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

Annotation-based subscriptions and event priority (ModTheSpire) #34

Open
daviscook477 opened this issue Mar 28, 2018 · 2 comments
Open
Labels
enhancement New feature or request
Projects

Comments

@daviscook477
Copy link
Owner

daviscook477 commented Mar 28, 2018

Right now the subscription system is messy and results in large and confusing bits of code like:

@SpireInitializer
public class FruityMod implements PostInitializeSubscriber,
    EditCardsSubscriber, EditRelicsSubscriber, EditCharactersSubscriber,
    EditStringsSubscriber, SetUnlocksSubscriber, OnCardUseSubscriber,
    EditKeywordsSubscriber, OnPowersModifiedSubscriber, PostExhaustSubscriber,
PostBattleSubscriber, PostDungeonInitializeSubscriber, PostDrawSubscriber {

These pieces of code define all the events that a class is listening for but are all bunched at the top of the file and make it difficult to know which methods are actually listening for events.

Instead we should take a look at the manner in which Minecraft Forge (link) handles this issue. Basically this means rather than having a class implement an interface the class will declare its intent to be a subscriber to events with the @Subscriber annotation and then it can annotate any of its methods with event annotations that signal to BaseMod that the specific method will be listening for the event. Here's an example of the idea:

@Subscriber
public class MyMod {
    @EditCardsEvent
    public static void handleEditCards() { /* do stuff in here */ }
}

Every method that has an Event annotation would need to make sure it has the proper types for each of the parameters it is being passed and then ModTheSpire would parse mods when they are loaded for the annotations to build up a list of which classes need which events.

Additionally we will need a standardized way to fire events, preferably through some static method.

When using the Event annotations preferably rather than just providing an indication of what events the class wants to listen to we could also implement a priority system. So rather than just implementing an unordered set of subscribers to each event we would use an ordered list, or perhaps some kind of tree map to maintain an ordering of the events through a priority system that allows event order to be specified. An example would look like this.

@Subscriber
public class ExhaustIndicator
    @PostExhaustEvent(priority=2)
    public static void handleExhaust(AbstractCard card) {
        System.out.print("A card");
    }

    @PostExhaustEvent(priority=1)
    public static void handleExhaust(AbstractCard card) {
        System.out.println(" was exhausted!);
    }
}

Using the priority system the above example would always print "A card was exhausted!" in that order rather than having the order to indeterminate.

@daviscook477 daviscook477 added the enhancement New feature or request label Mar 28, 2018
@daviscook477 daviscook477 changed the title Annotation-based subscriptions Annotation-based subscriptions and event priority Mar 28, 2018
@daviscook477 daviscook477 added this to To do in Roadmap Mar 29, 2018
@daviscook477 daviscook477 moved this from To do to In progress in Roadmap Mar 29, 2018
@daviscook477
Copy link
Owner Author

Implementation here: https://github.com/daviscook477/ModTheSpire/tree/feature/event-bus

Still needs:

  • an analog of unsubscribeLater to avoid ConcurrentModificationExceptions when trying to unregister and event while in the handler for it
  • to be tested
  • to use javassist for annotation checking rather than java because that prevents SlayTheSpire.exe from working

@daviscook477 daviscook477 changed the title Annotation-based subscriptions and event priority Annotation-based subscriptions and event priority (ModTheSpire) Mar 30, 2018
@daviscook477
Copy link
Owner Author

Things to test:

  • multiple methods in the same class with different names that handle the same event
  • instance methods
  • the unregister system

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Roadmap
  
In progress
Development

No branches or pull requests

1 participant