Re: [Ohrrpgce] SVN: james/13137 Attackers doing "unhide" attacks are allowed to ignore the hidden status
Haha! I guess some more clean-up would be good, but mostly I am just glad to have fixed the two most common cases of a simple jump/land chain getting a hero or enemy completely stuck. On Tue, Oct 18, 2022, 9:08 PM Ralph Versteegen via Ohrrpgce < ohrrpgce@lists.motherhamster.org> wrote: > Wow there are a huge number of edge cases and special cases here, I regret > looking into this... > > I see the problems were introduced in: > r12685 "Fix "Jump" (and the other new hiding attacks) to properly prevent > targetting when used by heroes". > Yep, that must have been when I broke it r12874 "Fix check_for_unhittable_invisible_foe() to properly handle > BattleSprite.hidden" > But maybe one of the problems with these is that they only allowed > targeting self when hidden with an exception for "Self" target class > attacks, not attacks in general that target self. > A broader change would be to allow all attacks that target self (regardless > of Target Class) to ignore hidden-untargetability. But is that the correct > thing to do? For example if a hero is hidden and then uses a whole-party > heal. > I just wanted to fix the case where self is the only target, but the attack fails completely because of no targets, so the land/unhide never happens. I was trying to stick narrowly to that exception > I think you only fixed part of the problem with r12874. > check_for_unhittable_invisible_foe used to start with: > IF bslot(target).vis THEN RETURN NO > .vis got split into .vis and .hidden so you added > IF bslot(target).hidden THEN RETURN YES > but that's the wrong way around, shouldn't it actually have been > IF bslot(target).vis ANDALSO bslot(target).hidden = NO THEN RETURN NO > But I guess the goal was to fix part of > https://github.com/ohrrpgce/ohrrpgce/issues/1233, not just replicate the > old behaviour, by being stricter. > The new logic (even after this r13137) disallows self-targetting on-death > attacks or ones hitting dead targets, when the target is hidden, but both > used to be allowed. If > I am a little concerned if I might have broken self targetting on-death attacks that might have worked previously. I can re-check those cases > > Hmm, also, it looks to me like before r12685, get_valid_targs let all > spread attacks target hidden targets but check_for_unhittable_invisible_foe > would then remove them unless one of the special cases (can hit dead or > self-bequesting on-death) applied, but now it never gets as far as checking > those special cases. You've added a shared > should_enforce_hidden_untargetability function which is a step in the right > direction but it doesn't include the old special cases. > > > "This fixes jump-land chains when they jump on self, or jump on another > jumper" > But you can only jump on a jumper that hasn't jumped yet when you target > the attack, right? > Yes, but this turns out to be surprisingly easy to do in battles with more than one jumper. (See https://github.com/ohrrpgce/ohrrpgce/issues/1234) I assume that's the > reason for adding this exception. Although it could make visual sense to > allow Jump/Hide attacks on hidden targets (once bug 1234 is somehow fixed > to deal with the targetting cursor), it's not currently allowed.And when > 1234 is fixed we can add attack target settings to allow hidden targets, > but in the meantime we have all these undocumented special cases for > allowing it. > > Also I wonder whether target-class Self attacks should override "Can't > target hero/enemy slot X", since it overrides all other kinds of > untargetability, but maybe it's better to avoid making changes. > I don't know. I don't think those "can't target" bits can change mid-battle like hidden status can, so it is less concerning > BTW, we have atkrAnim* constants for attack.attacker_anim. Wish we had > ones for the target classes. > I wondered about that but failed to search hard enough for them :D > > On Tue, 18 Oct 2022 at 12:13, James Paige via Ohrrpgce < > ohrrpgce@lists.motherhamster.org> wrote: > >> This fixes a relatively recent breakage of Jump/Land and Hide/Unhide >> which is why it isn't mentioned in the whatsnew >> >> Although I do think the new changes could have also fixed some old >> incorrect edge-cases with jump/land, since it is a little more explicit now >> about allowing land/unhide to ignore the hidden status of the target >> >> On Mon, Oct 17, 2022 at 7:10 PM subversion--- via Ohrrpgce < >> ohrrpgce@lists.motherhamster.org> wrote: >> >>> james >>> 2022-10-17 16:09:13 -0700 (Mon, 17 Oct 2022) >>> 188 >>> Attackers doing "unhide" attacks are allowed to ignore the hidden status >>> of their targets. >>> Fixes broken battles when jump/land or hide/unhide chain is >>> self-targeted, or jumping on a jumper >>> --- >>> U wip/bmod.rbas >>> U wip/bmodsubs.bas >>> U wip/bmodsubs.bi >>> >>> ___ >>> Ohrrpgce mailing list >>> ohrrpgce@lists.motherhamster.org >>>
Re: [Ohrrpgce] SVN: james/13137 Attackers doing "unhide" attacks are allowed to ignore the hidden status
Wow there are a huge number of edge cases and special cases here, I regret looking into this... I see the problems were introduced in: r12685 "Fix "Jump" (and the other new hiding attacks) to properly prevent targetting when used by heroes". r12874 "Fix check_for_unhittable_invisible_foe() to properly handle BattleSprite.hidden" But maybe one of the problems with these is that they only allowed targeting self when hidden with an exception for "Self" target class attacks, not attacks in general that target self. A broader change would be to allow all attacks that target self (regardless of Target Class) to ignore hidden-untargetability. But is that the correct thing to do? For example if a hero is hidden and then uses a whole-party heal. I think you only fixed part of the problem with r12874. check_for_unhittable_invisible_foe used to start with: IF bslot(target).vis THEN RETURN NO .vis got split into .vis and .hidden so you added IF bslot(target).hidden THEN RETURN YES but that's the wrong way around, shouldn't it actually have been IF bslot(target).vis ANDALSO bslot(target).hidden = NO THEN RETURN NO But I guess the goal was to fix part of https://github.com/ohrrpgce/ohrrpgce/issues/1233, not just replicate the old behaviour, by being stricter. The new logic (even after this r13137) disallows self-targetting on-death attacks or ones hitting dead targets, when the target is hidden, but both used to be allowed. If Hmm, also, it looks to me like before r12685, get_valid_targs let all spread attacks target hidden targets but check_for_unhittable_invisible_foe would then remove them unless one of the special cases (can hit dead or self-bequesting on-death) applied, but now it never gets as far as checking those special cases. You've added a shared should_enforce_hidden_untargetability function which is a step in the right direction but it doesn't include the old special cases. > "This fixes jump-land chains when they jump on self, or jump on another jumper" But you can only jump on a jumper that hasn't jumped yet when you target the attack, right? (See https://github.com/ohrrpgce/ohrrpgce/issues/1234) I assume that's the reason for adding this exception. Although it could make visual sense to allow Jump/Hide attacks on hidden targets (once bug 1234 is somehow fixed to deal with the targetting cursor), it's not currently allowed.And when 1234 is fixed we can add attack target settings to allow hidden targets, but in the meantime we have all these undocumented special cases for allowing it. Also I wonder whether target-class Self attacks should override "Can't target hero/enemy slot X", since it overrides all other kinds of untargetability, but maybe it's better to avoid making changes. BTW, we have atkrAnim* constants for attack.attacker_anim. Wish we had ones for the target classes. On Tue, 18 Oct 2022 at 12:13, James Paige via Ohrrpgce < ohrrpgce@lists.motherhamster.org> wrote: > This fixes a relatively recent breakage of Jump/Land and Hide/Unhide which > is why it isn't mentioned in the whatsnew > > Although I do think the new changes could have also fixed some old > incorrect edge-cases with jump/land, since it is a little more explicit now > about allowing land/unhide to ignore the hidden status of the target > > On Mon, Oct 17, 2022 at 7:10 PM subversion--- via Ohrrpgce < > ohrrpgce@lists.motherhamster.org> wrote: > >> james >> 2022-10-17 16:09:13 -0700 (Mon, 17 Oct 2022) >> 188 >> Attackers doing "unhide" attacks are allowed to ignore the hidden status >> of their targets. >> Fixes broken battles when jump/land or hide/unhide chain is >> self-targeted, or jumping on a jumper >> --- >> U wip/bmod.rbas >> U wip/bmodsubs.bas >> U wip/bmodsubs.bi >> >> ___ >> Ohrrpgce mailing list >> ohrrpgce@lists.motherhamster.org >> http://lists.motherhamster.org/listinfo.cgi/ohrrpgce-motherhamster.org >> > ___ > Ohrrpgce mailing list > ohrrpgce@lists.motherhamster.org > http://lists.motherhamster.org/listinfo.cgi/ohrrpgce-motherhamster.org > ___ Ohrrpgce mailing list ohrrpgce@lists.motherhamster.org http://lists.motherhamster.org/listinfo.cgi/ohrrpgce-motherhamster.org
Re: [Ohrrpgce] SVN: james/13137 Attackers doing "unhide" attacks are allowed to ignore the hidden status
This fixes a relatively recent breakage of Jump/Land and Hide/Unhide which is why it isn't mentioned in the whatsnew Although I do think the new changes could have also fixed some old incorrect edge-cases with jump/land, since it is a little more explicit now about allowing land/unhide to ignore the hidden status of the target On Mon, Oct 17, 2022 at 7:10 PM subversion--- via Ohrrpgce < ohrrpgce@lists.motherhamster.org> wrote: > james > 2022-10-17 16:09:13 -0700 (Mon, 17 Oct 2022) > 188 > Attackers doing "unhide" attacks are allowed to ignore the hidden status > of their targets. > Fixes broken battles when jump/land or hide/unhide chain is self-targeted, > or jumping on a jumper > --- > U wip/bmod.rbas > U wip/bmodsubs.bas > U wip/bmodsubs.bi > > ___ > Ohrrpgce mailing list > ohrrpgce@lists.motherhamster.org > http://lists.motherhamster.org/listinfo.cgi/ohrrpgce-motherhamster.org > ___ Ohrrpgce mailing list ohrrpgce@lists.motherhamster.org http://lists.motherhamster.org/listinfo.cgi/ohrrpgce-motherhamster.org
[Ohrrpgce] SVN: james/13137 Attackers doing "unhide" attacks are allowed to ignore the hidden status
james 2022-10-17 16:09:13 -0700 (Mon, 17 Oct 2022) 188 Attackers doing "unhide" attacks are allowed to ignore the hidden status of their targets. Fixes broken battles when jump/land or hide/unhide chain is self-targeted, or jumping on a jumper --- U wip/bmod.rbas U wip/bmodsubs.bas U wip/bmodsubs.bi ___ Ohrrpgce mailing list ohrrpgce@lists.motherhamster.org http://lists.motherhamster.org/listinfo.cgi/ohrrpgce-motherhamster.org
