Better offsets handling for gameplay props.

[Sword352]

Currently, Funkin directly changes the x and y properties for offsets. This is a bit tedious because it can break stuff, such as tweens.

This pull request changes the way animation offsets are handled without changing x, y or offset.
Ideally this could be implemented on other classes too.

[EliteMasterEric]

I'm bewildered that this doesn't immediately break things, especially since you deleted a bunch of code that was supposed to keep Week 6 working, but base game characters do seem to work during my playthrough.

My main concerns are:

• Further playtesting of this change with mods
• Better variable naming because I got very confused thinking animOffset and animOffsets were the same (it should also be private so people don't accidentally edit it).
• Moving this code to Bopper.hx; you are overriding the behavior of stage props in the character class, and by doing so making characters and stage props behave differently.

[Sword352]

I wouldn't be doing a pull request if I haven't tried, of course!

Re-tried the changes I did with some mod songs and seems to be working as excepted (sorry for the poor video quality).

video.mp4

[Raltyro]

Sword it would be cool if you make it compatible with angles using the fast trigonometries function from FlxMath

final rad:Float = angle * FlxAngle.TO_RAD;
output.x -= animOffset.x * FlxMath.fastCos(rad) - animOffset.y * FlxMath.fastSin(rad);
output.y -= animOffset.y * FlxMath.fastCos(rad) - animOffset.x * FlxMath.fastSin(rad);

something like this, i probably screwed something writing this in phone but yeah

[BlueColorsin]

review bugged

Sword it would be cool if you make it compatible with angles using the fast trigonometries function from FlxMath

final rad:Float = angle * FlxAngle.TO_RAD;
output.x -= animOffset.x * FlxMath.fastCos(rad) - animOffset.y * FlxMath.fastSin(rad);
output.y -= animOffset.y * FlxMath.fastCos(rad) - animOffset.x * FlxMath.fastSin(rad);

something like this, i probably screwed something writing this in phone but yeah

shouldn't this have a

if(angle != 0) {
    //code
}

???

[Sword352]

Can't get it working, perhaps in another pr?

[Raltyro]

idea from blue: normalized scale?

[EliteMasterEric]

Still looking for clarification on this one

[EliteMasterEric]

Can you give an explanation as to how/why your implementation doesn't require this custom scale logic?

[Sword352]

To be honest I'm not quite sure why it was there in the first place.
People says this somewhat makes the character progressively "fly" because scale isn't applied on value.
If this was meant for week 6 then I don't see how that has an effect as it just scales then divide by 6. However if this is meant to normalize the offset by scale (which I thought funkin doesn't) then I can re-implement that back.

[gamerbross]

I love this solution, but it has an issue, the base game system somehow ignores the idle offsets, no matter what offset you put in idle, it will behave as it doesnt have any (but the rest of animations work just fine) so you would have to implement that.
(i think im not explaining correctly, lemme know)

[Sword352]

I love this solution, but it has an issue, the base game system somehow ignores the idle offsets, no matter what offset you put in idle, it will behave as it doesnt have any (but the rest of animations work just fine) so you would have to implement that. (i think im not explaining correctly, lemme know)

not sure I'm following... maybe re-explain it yeah

[gamerbross]

I realized this while porting VS Entity to the base game. In the mod, Nikusa has offsets in her idle animation (instead of position offset)

And I just did the same in the port, but putting an animation offset to idle, just does nothing and retargets all of the animation offsets to be 0, 0
(If idle offsets are [195, 501], then they "are changed" to be [0, 0] and (for example) singLEFT offsets were [181, 474], and they changed to [-14, -27] (got substracted by original idle ofssets))
So the problem with this implementation is that it doesnt have the same behavior, it just renders Nikusa at the original offsets
(isnt really your fault, more like a weird thing done in base game)
Resume: somehow you should make idle offsets to [0, 0] and substract idle original offsets to every animation offsets

[gamerbross]

whats the status of this? we really need it 🙏

[EliteMasterEric]

I was porting some mods this week and encountered the issues people have been having with offsets, so I'm looking at this PR now.

[EliteMasterEric]

I finally reviewed and approved this branch

Since this invalidates both #2332 and #2972, I manually implemented a fix for the scaling issue myself, and will be closing those issues.

[gamerbross]

could you show the fix? so I can use it meanwhile

[EliteMasterEric]

could you show the fix? so I can use it meanwhile

  function applyAnimationOffsets(name:String):Void
  {
    // I don't take globalOffsets into account here anymore, I moved that to getScreenPosition.
    var offsets = animationOffsets.get(name);
    this.animOffsets = offsets;
  }

  override function getScreenPosition(?result:FlxPoint, ?camera:FlxCamera):FlxPoint
  {
    var output:FlxPoint = super.getScreenPosition(result, camera);
    // Combine the offsets then apply scale before adding.
    output.x -= (animOffsets[0] - globalOffsets[0]) * this.scale.x;
    output.y -= (animOffsets[1] - globalOffsets[1]) * this.scale.y;
    return output;
  }

[gamerbross]

could you show the fix? so I can use it meanwhile

  function applyAnimationOffsets(name:String):Void
  {
    // I don't take globalOffsets into account here anymore, I moved that to getScreenPosition.
    var offsets = animationOffsets.get(name);
    this.animOffsets = offsets;
  }

  override function getScreenPosition(?result:FlxPoint, ?camera:FlxCamera):FlxPoint
  {
    var output:FlxPoint = super.getScreenPosition(result, camera);
    // Combine the offsets then apply scale before adding.
    output.x -= (animOffsets[0] - globalOffsets[0]) * this.scale.x;
    output.y -= (animOffsets[1] - globalOffsets[1]) * this.scale.y;
    return output;
  }

add the isPixel thing, it broke senpai offsets lmao

[gamerbross]

also characters with idle offsets break when restarting the song because of this

Just remove the + character.animOffsets[x]

[gamerbross]

and why substracting globalOffsets? the stage already does that, just causes it to duplicate it. I feel like it should change the position and not act like a visual offset

[gamerbross]

my code end to this so it works properly:

  override function getScreenPosition(?result:FlxPoint, ?camera:FlxCamera):FlxPoint
  {
    var output:FlxPoint = super.getScreenPosition(result, camera);
    // Combine the offsets then apply scale before adding.
    output.x -= animOffsets[0] * (!isPixel ? this.scale.x : 1);
    output.y -= animOffsets[1] * (!isPixel ? this.scale.y : 1);
    return output;
  }

[gamerbross]

it alsoooo breaks the spirit position due to idle offsets (-220, -280)

fixed offsets:

  [
    {
      "name": "idle",
      "prefix": "idle spirit_",
      "offsets": [0, 0]
    },
    {
      "name": "singLEFT",
      "prefix": "left_",
      "offsets": [20, 0]
    },
    {
      "name": "singDOWN",
      "prefix": "spirit down_",
      "offsets": [390, 390]
    },
    {
      "name": "singUP",
      "prefix": "up_",
      "offsets": [0, 40]
    },
    {
      "name": "singRIGHT",
      "prefix": "right_",
      "offsets": [0, 0]
    }
  ]

[gamerbross]

@EliteMasterEric sorry for mention but maybe you had notifications off on this PR

[KutikiPlayz]

I mean correct me if I'm wrong but the way I see all this isPixel bs is that initially when they were adding the scale multiplication, it broke pixel characters cuz they're scaled up, and instead of just changing the pixel character's offsets to work with the new system they just slapped a bandaid on it by hardcoding a "fix" for pixel characters, which just turns off the scale multiplication.
And I know they didn't change the offsets because if you look back at the legacy 0.2.x version, the spirit character offsets for example, are the exact same as they are now.

If it were me I'd just readjust the offsets in each of the pixel character files, and then you don't need any of that hardcoded bandaid fix.

With a sprite that in reality is, lets say 64x64, an animation offset of 30 would work at a higher scale like it originally did, but now that the offsets are scaled as well, that turns into 180, and then it's incorrect.
So just divide the old offset that used to work by the scale and boom you've now got better numbers that actually work for the small sprite.

spirits new working offsets would be this for example:

  [
    {
      "name": "idle",
      "prefix": "idle spirit_",
      "offsets": [0, 0]
    },
    {
      "name": "singLEFT",
      "prefix": "left_",
      "offsets": [3, 0]
    },
    {
      "name": "singDOWN",
      "prefix": "spirit down_",
      "offsets": [65, 65]
    },
    {
      "name": "singUP",
      "prefix": "up_",
      "offsets": [0, 7]
    },
    {
      "name": "singRIGHT",
      "prefix": "right_",
      "offsets": [0, 0]
    }
  ]

based on gamerbross's updated spirit offets which adjust for the removal of the idle offsets.

Now I do understand the fact that doing this would require everyone to readjust the offsets of any scaled character that they're porting to the game from elsewhere, but personally I see that as a minor inconvenience at best. Whoever made that mod porter can just automatically divide any animation offsets by the character scale in the program, and then boom. No issues for anyone.