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

Suggestion: rework of API to make Actions easier to deal with #22

Open
curz46 opened this issue Apr 5, 2018 · 1 comment
Open

Suggestion: rework of API to make Actions easier to deal with #22

curz46 opened this issue Apr 5, 2018 · 1 comment

Comments

@curz46
Copy link

curz46 commented Apr 5, 2018

Regarding this API usage:

builder.putElement(slot, new ActionableElement(
    new RunnableAction(container, ActionType.NONE, "", context -> {}),
    item
));

This is messy for a number of reasons:

  • There is no reason to provide the StateContainer to an Action - context can be passed when necessary.
  • Even if the ActionType is NONE, you still have to pass a goalState. Even though this could be made a default property of an Action, such that if only the container and effective consumer (context -> {}) is passed, it still isn't as clean as it could be. In reality, goalState logic is just another consumer permanently attached to the Action, when they could easily be merged together.

I propose:

ActionableElement::new(ItemStack, Consumer<ActionContext>)

Here, ActionContext replaces what was initially RunnableAction and replaces the functionality of the consumer, however here it is not the RunnableAction passed into the anonymous function - it is an object specifically for the runnable's context.

To replace the functionality of goalState transition being easily requested with ActionType I propose Actions.GO and Actions.BACK, where Actions is an interface consisting of static default Consumer<ActionContext which automatically change the state.

Actions.GO could be defined as state -> context -> context.setState(state), which allows the use of Actions.GO("some_state").

Actions.BACK could be defined context -> context.setState(context.getParent()) or just context -> context.back(). It is useful to shorten this, because of the following scenario.

new ActionableElement(item, context -> {
    context.getObserver().sendMessage(...);
    context.back();
});

This allows for the previous functionality where a runnable can be executed AND the state can be changed at once.

@codeHusky
Copy link
Owner

I'm a bit late, but this is def on the roadmap.

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

No branches or pull requests

2 participants