Tween instances interfering with each other

Question

Tween instances interfering with each other

bc-bytes opened this issue 5 years ago · 9 comments

I'm having an issue with this library. When I create multiple instances of a tween, they seem to be interfering with each other. Lets say I have a tween for the player in the player class:

pl={
 x=0,
 y=0,
 sprite=0
}
pl.tween=tween_machine:add_tween({
 func=inback,
 duration=0.45
})
pl.tween:register_step_callback(pl_update_y)
pl.tween:register_finished_callback(pl_tween_end)

That works as expected. However, when I try to update tweens created for enemy objects, the enemy movement (processed by the enemy tween) is causing the player to move erratically. When I remove the enemy tween the player tween works fine again. Here's how I create enemy objects:

enemy_group1={}
for i=1,max_group_enemies do
  enemy={
   etype=1,
   width=0,
   height=0,
   edir=⬅️,
   tween=tween_machine:add_tween({})
  }
add(enemy_group1,enemy)

Basically, each enemy has its own tween, and is updated each frame like this:

if e.state==1 then
  e.tween.v_start=135
  e.tween.v_end=-15
  e.tween.duration=4.27
  e.tween.func=linear
  e.state=2
 else
  e.x=e.tween.value
  if e.x<-8 then
   e.alive=false
   enemies_alive-=1
  end
end

The only difference between the two (player/enemy) tweens is that I use a callback for the player tween to update player position, but access the tween's value directly to update enemy positions.

I'm new to pico-8 development, so it's quite possible I'm misunderstanding something on the Lua side of things. Any help much appreciated, and thanks for the library!

Answer 1 · 2020-06-10T18:21:24.000Z

@bc-bytes Thanks for notifying me of this! It's been quite some time since I last played with any Lua or Pico-8 development, so I'm sorry but you may need to wait a couple of days for me to find some time after work to test this issue and provide a resolution. I understand that's not especially helpful for your development when you're trying to use the library for your project, but I'll try and aim to either have this fixed or let you know what you may be doing wrong before the weekend (or by tonight, if we're lucky). Thanks again.

Answer 2 · 2020-06-10T19:53:21.000Z

No worries, thanks for the reply. I've posted the issue up on the pico-8 forums as well to see if anyone else can provide any insights.

Answer 3 · 2020-06-10T22:17:31.000Z

@bc-bytes So I did a little refreshing on how metatables work to make sure I wasn't using them incorrectly, this may be one of the factors causing problems with your code.

I'll start by breaking down my understanding of how metatables work, so you can better understand how tween objects are created and constructed, so that you won't run into any unforeseen issues in the future!

As Lua doesn't really have the concept of "classes" to the extent of object instances sharing properties as well as methods, metatables are a great way of creating new objects that effectively act as you'd expect an instance to within any other programming language.. with some slight differences.

We create a Lua object as such, along with some functions we'd like it to be able to call:

__tween_base = {
        func = nil,
        v_start = 0,
        v_end = 0,
}

function __tween_base:register_step_callback(func)
    -- function body here
end

I imagine this syntax is familiar to you, as it's just your general way of creating object tables and assigning functions to them.

A nice trick we can do with metatables though, is after creating that base if we do:

function __tween_base:create_instance(instance)
    setmetatable(instance, {__index = __tween_base})
    return instance
end

my_instance = __tween_base:create({
    -- insert instantion paremeters here
})

What we're doing is basically saying "if a field is called on my_instance that doesn't exist, check the backing table assigned to __index". So for example, we try to call my_instance:register_step_callback, we didn't rewrite that function body inside the call to :create({}), we only declared the properties we wanted to assign. my_instance therefore doesn't have a declared field named register_step_callback we can call a function on, however, due to us assigning the the special __index property, when it checks the backing table, it will find the function and call it.

The trick part of this you may have noticed from the explanation there, is that it's only reading from the reference to the backing table. There isn't a brand new instance of all these functions and fields waiting to be used on our object. Which means that any writing we do to fields that haven't been explicitly declared in our object instantation is writing directly to our backing table, which is what will be causing overlap between multiple instances of the tweens.

So the solution here, is that any internal fields that are going to be at any point written to in an object instance should be declared in the instantiation to avoid problems.

This is mostly my fault however, as I didn't explain this in the readme, and my example code doesn't do this properly either.

There are better, more error-free methods of applying object this procedure to "class" instantiation, which shouldn't require as much reliance on user knowledge to work correctly.

Either tonight or tomorrow morning, I should have a new push updating the code to resolve this, as well as provide a more success-friendly explanation in the readme and example.

Thanks for bringing this to my attention! I'll ping you again when a fix has been pushed.

Answer 4 · 2020-06-12T23:39:14.000Z

@bc-bytes I just pushed an update to the https://github.com/JoebRogers/PICO-TweenMachine repo, apologies to the slight timeline delay, had to wait until after work today to run tests on the changes I made yesterday.

I believe the new changes should resolve your issue, as the fields for the __tween base object are now constructed in an anonymous local object within the add_tween function and appended to the passed in instance before being assigned to the metatable. This means that there will no longer be any issues with backing fields being overwritten, and it shouldn't change the way you've been creating your instance objects to pass into add_tween, meaning no tokens will be wasted declaring fields unnecessarily.

Please let me know if you get around to testing the updated code and I'll close the issue here. Also as a small aside, the issue here is actually in the TweenMachine repo, rather than this one, so future issues like this should be opened within that repo!

Thanks.

Answer 5 · 2020-06-13T00:05:52.000Z

Brilliant! Just tested, and it seems to work perfectly now. Many thanks for the fix.

Answer 6 · 2020-06-13T00:32:17.000Z

Awesome, glad to hear it helped! Closing issue.

Answer 7 · 2020-06-13T22:10:01.000Z

Unfortunately I think there might still be an issue with this. When I tested yesterday, enemy tweens were no longer affecting player tweens. However, today I added more enemies, each with their own tween, and I'm getting odd behaviour. Here's an enemy tween that is a property of an enemy object:

tween=tween_machine:add_tween({
 v_start=129,
 v_end=-9,
 duration=4.27,
 func=linear
})

I would expect the enemy to appear on the far right side of the screen and move towards the left of the screen. However, the actual behaviour is that the enemy appears in the middle of the screen then moves to the left.

Answer 8 · 2020-06-13T22:23:02.000Z

OK, after a bit more investigation, this time the error is on my side. I pre-define all my enemies in a table, but each of them are setup with a tween as a property. That means that all those tweens are being updated before my game actually starts. Doing a tween.restart() when the enemy is due to spawn fixed it. However, if I create 100 enemies, then all those enemy tweens are going to be updated constantly, even when the enemies are not being used. Maybe there should be an extra flag to tell the tween that it can start?

Answer 9 · 2020-06-13T22:43:31.000Z

Glad to hear it was just on your side, I just ran a few extra tests as I was worried there was a possibility of something going on with the registered functions not being local to the instance fields somehow, but couldn't replicate the problem.

This is actually more of a documentation/example issue than an API issue in my opinion, as the way to handle this currently is by setting the finished flag to true in the tween creation. The callback is only executed within the :update() function, after the logic has been checked and the finished field has been set to true. If it's already true when the function is run, it just early exits. Meaning you can set the flag to true in the object creation without worrying about setting off callbacks, then just call restart in order to start the tween.

The main reason for this was that the logic already exists in the code for the tween not to run, so keeping in mind the platform it's running on, I opted to not write a somewhat redundant additional function to wrap that logic at the expense of tokens.

I will add that info to the comments of the file, as well as add a small section on it to the readme.

Thanks for pointing it out! It's always difficult to read what parts of an API are going to cause issues for users, so it's definitely helpful to know what could be better documented, or what additional usecase examples could be provided to help people get set up correctly!