omf2097 / openomf

One Must Fall 2097 Remake
http://www.openomf.org
MIT License
372 stars 40 forks source link

Defeat status should be assessed on hit, not when HAR touches floor #290

Open gdeda opened 9 years ago

gdeda commented 9 years ago

All of throwing or moves which won't consist in a direct impact, rather in an opponent being throwed away, wont' trigger KO status: it continues to fight even if it reaches 0 health. I can confirm this at least on Jaguar Overhead Throw and the Thorn throw. In original OMF the KO is triggered just as the throwing move is performed by the player.

gdeda commented 9 years ago

Corrected the title due to the following findings: After some testings, looks like the bug is not related to throwing, but about the KO being triggered only when the opponent touches floor (as evidence, you can still move if you give a deadly punch/kick). I have smashed a enemy HAR with a Katana throw and the KO is triggered only if the opponent touches floor. Same with Jaguar overhead throw: only if the opponent touches the floor first, the KO is triggered. Otherwise the match just continues.

IIRC, the KO should be triggered at the succesful attack, not when the opponent touches floor...

gdeda commented 9 years ago

Ok, I think I have pinpointed the issue. Or at least I believe, as my tiny coding experience is about C# :)

The issue should be because, according to har.c, every time the HAR goes down, there is a series of checks about if the match should end, or not...

har.c, line 470: if(h->state != STATE_DEFEAT && h->state != STATE_FALLEN && h->health <= 0 && h->endurance <= 0 && player_is_last_frame(obj)) {

            h->state = STATE_DEFEAT;
            har_set_ani(obj, ANIM_DEFEAT, 0);
            har_event_defeat(h);
        } else if(pos.y >= (ARENA_FLOOR-5) &&
                  IS_ZERO(vel.x) &&
                  player_is_last_frame(obj)) {
            if (h->state == STATE_FALLEN) {
                if (h->health <= 0 && h->endurance <= 0) {
                    // fallen, but done bouncing
                    h->state = STATE_DEFEAT;
                    har_set_ani(obj, ANIM_DEFEAT, 0);
                    har_event_defeat(h);
                } else {
                    h->state = STATE_STANDING_UP;
                    har_set_ani(obj, ANIM_STANDUP, 0);
                    har_event_land(h);
                }
            } else {
                har_finished(obj);

What I understand here is, if the HAR goes down AND the health AND endurance is 0, ONLY THEN the defeat is called.

The HAR, anyway, won't die if health goes to 0 but there is some endurance, what instead happens is the next shot should bring endurance to 0.

har.c, line 519 int oldhealth = h->health; if(h->health <= 0) { h->health = 0; } if (oldhealth <= 0) { // har has no health left and is left only with endurance. // one hit will end them h->endurance = 0;

What's the issue then? That the endurance regen is not blocked at the next hit, and if the HAR gets a shot with 0 health BUT some time passes enough to restart endurance regen (for example, if the HAR was shot at high jump, OR if it is "busy" hugging the wall), then some endurance is recovered, and the match won't be over.

I have right now tested the same thing in OMF 2.1 and I don't see why it was coded so in OpenOMF: in OMF, even if the HAR has some health, the shot which will bring it to 0 causes endurance to go to 0 and defeat (and time slowed down). Was there a reason in particular? Talking about "oldhealth"?

A workaround would be to comment some lines out there AND to block endurance regen when health goes to 0. The optimal solution would be the defeat status to be assessed on hit, and not when the HAR touches floor...

Vagabond commented 9 years ago

This is certainly a bug.

Please feel free to work on a fix. One thing to be aware of is that a har with 0 health but > 0 endurance is still in a playable state, and endurance can regenerate in that case. I agree that if health is 0 and endurance is 0, there should be no regeneration and the match will end once the 'dead' HAR lands.

gdeda commented 9 years ago

Understood... my point, though, is why the "oldhealth" thing is there... was OMF behaving different before? In OMF 2.1 the condition a har with 0 health and > 0 endurance never happens... because the har is considered defeated "on hit" as health goes to 0, no matter endurance (which goes to 0 as health reaches 0).

A solution like that (admitted I can do that ^_^ ) would much likely simplify code instead to add blocks and other checks, and being much similar to OMF.

Otherwise, the workaround about removing the oldhealth things and block endurance regen also can do the trick..

If, instead, the oldhealth thing was reflecting a different behaviour from an early version of OMF, the code should be kept, and we should think for an option in a "special OpenOMF menu" to switch this

Vagabond commented 9 years ago

No, if the 'oldhealth' stuff is wrong, it is wrong. We don't have access to the original OMF source, so we're implementing the game logic based on observation of the original as well as reverse engineering.

If openomf doesn't do what OMF 2.1 does, we need to fix that. We use OMF 2.1 as our reference implementation and don't care what the older versions of the original did.

Vagabond commented 9 years ago

I think the 'endurance restore' code in har_tick is wrong, it should not regenerate endurance if endurance and health is 0.

Vagabond commented 9 years ago

Try it now, we figured out that endurance is floating point and we've changed how quickly it recharges. Probably still not perfect though and I think we can still clean up that 'oldhealth' code.

gdeda commented 9 years ago

Tried, it is nice! Although the regen rate now is much slower than in OMF (at least by 2 or 2.5x). Some tweakings should be done.

About the effect of this edit on the issue, yes it works (something else should be done, I can work on at least one aspect), but what I have right now seen in OMF is the "YOU WIN" can appear when the defeated HAR is mid-air. If we could be able to move the "trigger the check when HAR goes down" to "trigger the check when HAR is hit", that would resolve everything and no need to block regen.

In the meanwhile, I'm going to work about removing the part about HAR with 0 health and some endurance left . Expect a PR soon :)

gdeda commented 9 years ago

Changed title for clarity. This could require a not very small system overhaul...

katajakasa commented 7 years ago

Things still open:

gdeda commented 7 years ago

Defeat status should be assessed on hit, not when HAR hits ground. This seems to be a valid difference to the original game. The thing is, do we care ? Is this important ?

It is for sure a difference, but the accepted PRs made it low priority - now that endurance is set to 0 and blocked when health reaches 0, the situation the "dead HAR" could revive for a last stand while mid-air simply can't be. Fixing this could anyway prevent possible bugs which can happen during the "dying HAR fly"... athough I believe requires a nice overhaul of the system of how winning status is determined.