search

Terasology / Thirst

This module introduces Thirst and various related effects.
0 stars 8 forks source link

Rework the relationship between Thirst (and other systems) and Health #8

Open Cervator opened 5 years ago

Cervator commented 5 years ago

With #7 you can now die of thirst, yay! ... I think?

The discussion in the PR is well worth a read as it uncovered some underlying issues: exactly how should thirst and similar effects actually affect health? The way it is implemented now would for instance block healing from any source of healing due to the interception of the BeforeHeal event. In some cases that may make sense, like natural generation may stall due to dehydration. In other cases like a cleric casting a healing spell ... probably not.

One suggestion would be to improve the overall natural regeneration system, likely as part of an overhaul and extraction of Health from the Core module. Right now there is just the generic "Healing" but really there should probably be a separate system for natural regeneration that leads to a healing effect. Then Thirst, Hunger, and similar would just slow or stall the regeneration system and healing would never trigger. Other healing source in that case would be unaffected yet you could explicitly make effects that enhanced natural regeneration separately.

There have been efforts in the past to work a more advanced anatomical system (see the Anatomy) module, even one that isn't based on the typical hit point system. Natural regeneration could work in both cases, just the way the system is expressed (refilling hit points or scabbing over a wound to stop bleeding) would differ.

Developing such a system is beyond the scope of this issue and module, but I'm leaving this note here to go seek out other related modules and discuss with others here and on chat.

darshan3 commented 5 years ago

Related issue: Since foodConsumed() and drinkConsumed() are at the same priority level for Activate Event, the one that catches the event first wins and consumes the event. Hence Fruits can either satiate hunger or thirst but not both.

ktksan commented 4 years ago

Related issue: While trying to add an AffectThirstEvent (event sent out for other modules to react to and modify Thirst decay) , we encountered an issue/confusion regarding the Thirst Decay system. The issue being: we have no idea how the thirst decay system is working. The only line which tries to decay thirst is : https://github.com/Terasology/Thirst/blob/5a4129b0d147982faee3ecf953d84cfc2f010cc8/src/main/java/org/terasology/thirst/ThirstAuthoritySystem.java#L100-L101 Now, this method is only triggered every 60 seconds but the thirst decay can be clearly noticed in a sub-second interval. related discussion can be found here: https://github.com/Terasology/Thirst/pull/17

(Quoting @skaldarnar who in turn quotes @jdrueckert )

I fear fixing this turns out to be more elaborate work than initially planned 🙄 I'm tending towards the steps proposed by @jdrueckert: don't differentiate between running and walking (likely without effect anyways) reduce the interval of the periodic event (align with Hunger) only update the thirst value in a single place (taking elapsed time into account) add proper documentation for both system