[?][BUG][PL] Sound for failed assignment plays for successfully queued shimmier

[Simon]

NL-CE 1.0.1.

Repro:

• Have a level with climbers, shimmiers, and a ceiling to which we can climb.
  
• Make a lemming climb the wall.
  
• Select shimmier in the panel.
  
• Near the ceiling (but before he reaches it), hover over the climber and click.
  
• You'll hear the sound for a failed assignment.
  
• Wait for the climber to reach the ceiling.
  
• The lemming will shimmy. Edit: You'll also hear the sound for a successful assignment.
  

Observed: Sound for the failed assignment in step 5 and successful shimmying in step 7.

Expected instead: No sound in step 5 when we click. (Reason: The click produces an effect, it queues a shimmier, therefore it was successful.) Still successful shimmying in step 7.

-- Simon

[WillLem]

Do you hear the successful assignment sound when the lemming begins shimmying? I'm afk at the moment but, from memory, I think that queued assignments play the successful assignment sound when the skill action begins.

[Simon]

Yes, the sound for a successful assignment plays when the shimmying begins (in step 7).

-- Simon

[namida]

The problem with this is that a queued skill may still end up failing (if the queueing expires before the skill is able to be assigned). The only way around this would be to simulate this duration ahead every time a skill is assigned - which is doable, and code to simulate ahead already exists, but it is not quite 100% accurate; in particular, simulations do not take into account other lemmings besides the one being simulated, for the most part (one exception is that blockers still affect the simulated lemming), so for example, in the case of the Climber->Shimmier transition, this may be detected as a pass because the climber reaches the ceiling within the timeframe, but the assignment ends up failing because a bomber that was about to explode (which the simulation does not account for), does so (and removes the ceiling) inbetween the shimmier being queued and the time where it would have taken effect. It would still be an improvement over status quo, but there are edge cases it would miss (and depending how it's implemented, it may end up not playing a sound at all in these cases).

[Simon]

Ah, then queuing is more prevalent than I thought, and most queues fizzle without effect. If we want to fix the OP behavior, we'll have to distinguish now what will fizzle, and we've never had to distinguish before.

I have no good suggestion (other than your simulation) that fixes this exact problem and keeps all other failed sounds.

The only time I need explicit notification for assignment failure is: The click fails to assign because that new assignment would overwrite an existing same-tick assignment to different lemming. I don't need sounds for any other failed assignments.

In this light, I'm even beginning to think that the other failed assignment sounds are a problem. I didn't realize this before. For the same-frame problem, I'd like explicit feedback that the new assignment fails because of the existing same-frame assignment. The CE 1.0.1 fail sound doesn't tell me that; it merely tells me that there was some whatever reason to fail.

-- Simon

[WillLem]

IMHO, the bug here is not the assign fail sound (which correctly plays because the skill cannot be assigned on this frame), but the queued assignment itself. It's always felt like buggy behaviour to me (although I do generally choose to play with it toggled on for the few times it's useful vs. the times I have to rewind and clear the queue).

To be clear, I'm not suggesting that we get rid of queued assignments (just in case anyone is thinking that's what I'm getting at), but rather that we don't necessarily need to do anything here.

Reasoning:

1) The assign fail sound is not a bug; the assignment cannot happen on this frame, so feedback is correct. I see no reason to mess with that tbh.

2) When the assignment happens, the assignment sound plays. The user is provided with feedback correctly in both the attempted assignment (where it fails) and its eventual happening (where it happens only because queued assignments are a thing); all good. If anything, the fact that both sounds are played adds further clarification to exactly what's happened; the queued assignment is a courtesy to prevent the player having to click again at the point that the assignment becomes possible. In that light, the previous click (that created the queued assignment) is arguably irrelevant, at least for the purposes of providing immediate feedback.

Perhaps for shimmiers specifically, we could simply treat the assignment as not failed if it's made within the allowed climber>shimmier range, and play the assignment sound at the point of attempt. The only issue with this is that ideally the skill would also be deducted at the point of assignment, and since that messes with physics & replay timing, it would render CE incompatible with NL 12.14 content. So, we probably shouldn't mess with that either.

If anything, the only thing that may need to be addressed is your point here:

For the same-frame problem, I'd like explicit feedback that the new assignment fails because of the existing same-frame assignment. The CE 1.0.1 fail sound doesn't tell me that; it merely tells me that there was some whatever reason to fail.

Perhaps, along with making the assign fail sound optional, we could also allow the user to specify different sounds for different assignment fail types. It's one of the reasons I want to play the steel sound when the assignment fails because steel, but this was ruled out as undesirable. If we allow a fully customisable sound set, we can potentially allow different sounds for different fail types.

The problem would then be determining why the assignment has failed code-side: this is not trivial. I'd probably need to see significant community support for the idea before embarking on it. There's already plenty to be getting on with CE-wise so it'd be a fairly low priority at present.

[namida]

One thing to keep in mind is that the Shimmier is somewhat unique in having situations that require frame-perfect assignments. Nepster designed it this way (I did the same with eg. the Slider for consistency); queueing was already a thing at the time, so I believe Nepster felt it was acceptable to require the frame-perfect assignments precisely because queueing gave a generous window to "assign" it anyway.

For SLX, I would even go as far as suggesting changing this behavior - allow Climber->Shimmier whenever the climber is less than one animation cycle away from hitting his head (internally: there is at least one solid pixel somewhere between (lemming Y - head check offset - distance climber ascends in one animation cycle), and (distance climber ascends in one animation cycle - 1) below that). Slider->Shimmier is trickier, but perhaps during the last few frames, or alternatively, introduce a "dangler" state which it can be assigned during. Obviously such a change is out of the question for NLCE.

The problem would then be determining why the assignment has failed code-side: this is not trivial. I'd probably need to see significant community support for the idea before embarking on it. There's already plenty to be getting on with CE-wise so it'd be a fairly low priority at present.

It's not trivial, but I don't think it's quite as hard as you're thinking. From memory: All attempts to assign skills go through functions that return either True or False to indicate if the assignment passed or failed. So, the way I'd approach this is - instead of returning a Boolean, make these functions return an enum, with values for the various failure types (and one for success, of course). You could also use integers and constants, but an enum is a bit more bug-resistant, especially if for whatever reason you want to change the order of the possible values later.

I'd generally approach a change like this by first making the new code use the Enum and return the correct values of it (so, I'd replace all cases of "Result := false" in the relevant functions with the specific Enum value rather than a "generic fail"), but otherwise do exactly the same thing the old code did - essentially the Enum is just a fancy Boolean where one value means true, any other value means false. Once that works, I start writing code that actually does something with the new Enum values. Although this would change the physics code, assuming there's no bugs it won't change the actual physics (as it doesn't change what's reported as a success vs failure, it only adds more detail to the failures), so wouldn't be an NLCE-incompatible change.

[WillLem]

For SLX, I would even go as far as suggesting changing this behavior - allow Climber->Shimmier whenever the climber is less than one animation cycle away from hitting his head
...
Slider->Shimmier is trickier, but perhaps ... introduce a "dangler" state which it can be assigned during.

Way ahead of you on both counts. In SLX, Shimmiers can now be assigned to Climbers at any time (even if the shimmy action itself would fail, it remains useful as a cancelling/turning action), and the Dangler state was one of the earliest additions to SLX (implemented primarily to aid Shimmier assignability to Sliders in Classic Mode). Interesting that you'd come up with the exact same idea, up to the point of giving the state the same name; it certainly seems the most natural thing for the Slider to do.

It's not trivial, but I don't think it's quite as hard as you're thinking ... the way I'd approach this is - instead of returning a Boolean, make these functions return an enum, with values for the various failure types (and one for success, of course).

Yes, an enum or record is probably the best way go with this for sure, but I'd still want to see significant support for the idea before starting with it. Having many new sounds can be off-putting for some users, even if those sounds provide necessary clarification and feedback.

I suppose for future-proofing and code-streamlining purposes, the enum could be implemented anyway (even without the additional sounds). If doing this, I'd probably want to completely refactor the assigning of skills in general. Currently, there are 3 (or 4?) separate methods/functions that determine the assigning of a skill. I'm sure these have become necessary as skill assignment has become more complex (due to the replay system, highlit lems, etc), but the code has become more cumbersome to work with as a result. The assign fail sound, in particular, has had to go through several revisions and re-writes to get it to play nicely with the existing skill assignment system.

Ideally, a single method would take care of the whole thing, perhaps with functions to determine things like lemming state, priority, etc. But the result of any enum related to skill assignment should be easily accessible from a single place. So, there would be a lot of work to do. It's one for the long-term to-do list.