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

Items are not consumed after use #4

Open
oniatus opened this Issue Dec 18, 2016 · 3 comments

Comments

@oniatus
Contributor

oniatus commented Dec 18, 2016

Tested with latest cooking and hunger on PR#3:

  • give yourself a consumable item, e.g. an AppliePie
  • set your hunger to 10 (hungerSet 10 command)
  • eat the pie
  • the pie is not consumed.

By a short look, the HungerAuthoritySystem consumes the ActivateEvent and sends a FoodConsumedEvent instead. I found no usage of the FoodConsumedEvent though.
The ItemAuthoritySystem is responsible for removing consumable items on use but has a low event priority for ActivateEvent and never receives the event because it is consumed before.

Likely that this can be fixed easily by removing the event.consume() in the HungerAuthoritySystem but there may be a reason why we consume the event in the first place.


Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@Josharias

This comment has been minimized.

Show comment
Hide comment
@Josharias

Josharias Dec 19, 2016

Contributor

@oniatus, thank you for doing the investigation into this. It looks like I did this pattern only half correctly when I implemented drinkable and fillable cups in JS. The complexity that I needed to overcome, and why I mistakenly added the event consuming, was to allow only one interaction happen when activating a cup. With a cup, you could: drink the contents, dump the liquid into a tank, fill the cup from a water source, or fill the cup from a tank. Without cancelling the event (with consume), all the actions could happen one after another, and depending on the order you could end up: filling the cup, drinking the cup, and emptying the cup all in the same ActivationEvent leaving you with the same empty cup that you started with.

(thinking out loud) I suppose the solution to this could be one of:

  1. Provide a different mechanism than consume to allow multiple systems to tell each other if an ActivationEvent has already been acted upon so that you could allow only one action per ActivationEvent.
  2. Move the item destroying code someplace else that is not dependent on the ActivateEvent completing without being cancelled.

Any other thoughts?

Contributor

Josharias commented Dec 19, 2016

@oniatus, thank you for doing the investigation into this. It looks like I did this pattern only half correctly when I implemented drinkable and fillable cups in JS. The complexity that I needed to overcome, and why I mistakenly added the event consuming, was to allow only one interaction happen when activating a cup. With a cup, you could: drink the contents, dump the liquid into a tank, fill the cup from a water source, or fill the cup from a tank. Without cancelling the event (with consume), all the actions could happen one after another, and depending on the order you could end up: filling the cup, drinking the cup, and emptying the cup all in the same ActivationEvent leaving you with the same empty cup that you started with.

(thinking out loud) I suppose the solution to this could be one of:

  1. Provide a different mechanism than consume to allow multiple systems to tell each other if an ActivationEvent has already been acted upon so that you could allow only one action per ActivationEvent.
  2. Move the item destroying code someplace else that is not dependent on the ActivateEvent completing without being cancelled.

Any other thoughts?

@nihal111

This comment has been minimized.

Show comment
Hide comment
@nihal111

nihal111 Mar 20, 2017

Member

@Josharias I have used the second solution you suggested here-#6. Have a look. :)
Also a similar fix for the Thirst module- Terasology/Thirst#2

Member

nihal111 commented Mar 20, 2017

@Josharias I have used the second solution you suggested here-#6. Have a look. :)
Also a similar fix for the Thirst module- Terasology/Thirst#2

@Cervator

This comment has been minimized.

Show comment
Hide comment
@Cervator

Cervator Mar 27, 2017

Member

So interestingly this now works - for Cooking module items in multiplayer. Oddly the Joshaberry from JS still does not disappear. Both are fine in single player.

Also oddly the apple pie fails to improve the player's thirst level, it just works for the hunger level.

In single player JS the Joshaberry is consumed but only fills up hunger, not thirst. In multiplayer JS it fills thirst but not hunger - that is mildly amusing in some bewildering fashion :D

Maybe something somewhere is consuming an event that had other stuff to do as well, and single vs. multi happens to be switching the order around or not triggering the first so only the second happens.

Maybe consuming a food item (or even drinking from a wooden cup) should re-trigger hunger and thirst specific events depending on which if any module is active, but not be consumed itself so any action can be attached?

Probably some of this works already. Mostly thinking out loud hoping it'll be useful.

Pinging @nihal111 and @xrtariq2594 for reference :-)

Member

Cervator commented Mar 27, 2017

So interestingly this now works - for Cooking module items in multiplayer. Oddly the Joshaberry from JS still does not disappear. Both are fine in single player.

Also oddly the apple pie fails to improve the player's thirst level, it just works for the hunger level.

In single player JS the Joshaberry is consumed but only fills up hunger, not thirst. In multiplayer JS it fills thirst but not hunger - that is mildly amusing in some bewildering fashion :D

Maybe something somewhere is consuming an event that had other stuff to do as well, and single vs. multi happens to be switching the order around or not triggering the first so only the second happens.

Maybe consuming a food item (or even drinking from a wooden cup) should re-trigger hunger and thirst specific events depending on which if any module is active, but not be consumed itself so any action can be attached?

Probably some of this works already. Mostly thinking out loud hoping it'll be useful.

Pinging @nihal111 and @xrtariq2594 for reference :-)

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