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

UseHoeEvent doesn't take event state into account #3

Open
SimonMeskens opened this issue Oct 28, 2015 · 10 comments
Open

UseHoeEvent doesn't take event state into account #3

SimonMeskens opened this issue Oct 28, 2015 · 10 comments

Comments

@SimonMeskens
Copy link

progwml6/HungerOverhaul#107

@SimonMeskens
Copy link
Author

I'll try to make a PR, so you don't need to do the work

@squeek502
Copy link

Should be as simple as adding something like:

if (e.getResult() != Result.DEFAULT || e.isCancelled())
    return;

to the top of UseHoeEventHandler.tillDirt

@SMEZ1234
Copy link
Owner

@SimonMeskens Thanks for pointing out the issue. I can't do anything right now, actually, because my computer died recently, so if you create a pull request that would be great.

@SimonMeskens
Copy link
Author

Gotcha, I'll try out what squeek suggests, play around with it a little bit to make sure it works out fine.

@SimonMeskens
Copy link
Author

Small update: This issue bubbled up to the top of my todo, so I'll be testing some stuff this weekend hopefully

@SimonMeskens
Copy link
Author

@squeek502 EB's hoe event is firing first, at which point I can't tell if it was dirt or grass. Ideas?

@squeek502
Copy link

That is determined by the priority of the event subscription. Other mods can use @SubscribeEvent(Priority.HIGH) to always fire before one of default priority (default @SubscribeEvent), so that's not an issue. All EB needs to worry about is that it respects the current state of the event (the state determined by event handlers that are fired before it), by reading the current state of the relevant event statuses (isCancelled() and getResult()) and only doing work if it makes sense to.

This change is just the first step towards compatibility with HO. After this, HO will need to add special compatibility for EB grass/dirt. See my comment in the HO issue:

Will need changes in both mods. EnhancedBiomes will turn its dirt into farmland regardless of the event state, so even if HO handles EB dirt, there's no way to have that stop EB from turning it into farmland anyway.

@SimonMeskens
Copy link
Author

@SimonMeskens
Copy link
Author

just a note: EB has custom dirt, grass AND farmland

@SimonMeskens
Copy link
Author

I lowered event priority on the default hoe-ing behavior. As a vanilla mimic, it should have as low of a priority as possible anyway, any event should surpass it. Seemed a bit cleaner than requiring elevation of Hunger Overhaul priority. The commit is in the PR.

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

No branches or pull requests

3 participants