increpare / PuzzleScript

Open Source HTML5 Puzzle Game Engine
MIT License
905 stars 160 forks source link

Move sound triggered for an object that does not move #974

Open david-pfx opened 1 year ago

david-pfx commented 1 year ago

Has something to do with properties and layers, I'm still trying to debug it. Repro case attached, courtesy 'cool nico' repro-nico-unexpected-sound.txt

david-pfx commented 1 year ago

Re source code engine.js the for loop starting at line 1152: I read this as:

For this location (position index), is there a movement sound event matching both an object found at this location and a movement found at this location?

It will cause a false trigger if there are two objects at a location, of which one matches by object and the other matches by movement.

Am I right?

increpare commented 1 year ago

having a glance at the code and commenting without checking what I'm saying is correct, in case it's of any help:

for (var i=0;i<state.sfx_MovementMasks[layer].length;i++) {
var o = state.sfx_MovementMasks[layer][i];
var objectMask = o.objectMask;
if (objectMask.anyBitsInCommon(sourceMask)) {
var movementMask = level.getMovements(positionIndex);
var directionMask = o.directionMask;
if (movementMask.anyBitsInCommon(directionMask) && seedsToPlay_CanMove.
indexOf(o.seed)===-1) {
seedsToPlay_CanMove.push(o.seed);
}
}
}

For this location (position index), is there a movement sound event matching both an object found at this location and a movement found at this location?

I would say "a movement found at this location" is possibly wrong - I'd assume it's matching for a movement directly on the collisionlayer of the object. To get the raw up/down/left/right/etc movement mask you have to do something like:

 var layerMovement = movementMask.getshiftor(0x1f, 5*layer);

( https://github.com/increpare/PuzzleScript/blob/master/src/js/engine.js#L1188 )

But with the seedsToPlay_CanMove code, because we're comparing with the whole movementMask directly, I'd assume that movementMask is no longer just a number representing a movement in the abstract, but already represents a movement within the context of all the collision layers.

Apologies if this is more confusing than elucidating...

Message ID: @.***>

david-pfx commented 1 year ago

This is code you should know way better than I could but no, I can't see it.

david-pfx commented 1 year ago

It seems that Clement Sparrow already found and fixed this bug. https://github.com/ClementSparrow/Pattern-Script/issues/22

His patch matches some of but not all current PS code.