Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Manual "step" and multiple Shifty instances #109

Closed
4ian opened this issue Feb 23, 2019 · 10 comments
Closed

Manual "step" and multiple Shifty instances #109

4ian opened this issue Feb 23, 2019 · 10 comments

Comments

@4ian
Copy link

4ian commented Feb 23, 2019

Hi there!

Thanks for this nice library! Looks really cool and @Wend1go is looking at adding support for it in GDevelop.
As it's a game engine, I have a few questions to see how we can best integrate the game engine and Shifty:

  • Can we instantiate multiple instances of Shifty? A "level" is represented by a "scene" in GDevelop, and multiple scenes can be existing at the same time. For example, you can play a scene, pause it and run a new scene "on top" of it (think of playing, then starting a combat in a RPG, or opening a menu).
    • Tweens can be running on objects in these different scenes, but we don't want to run tweens on objects in a "paused" scene. In other words, any tween running in the current scene must not interfere with other tweens.
  • Can we get rid of the default requestAnimationFrame, and instead call manually the function processTweens? I've seen it's exported in tweenable.js, but not part of the public API?
    • I've seen Tweenable.setScheduleFunction and Tweenable.now too. Tweenable.now could be redefined to return the time of the game engine (so that tweens are following any time scaling/pausing applied to the game), but I'm not sure about setScheduleFunction. We would need an inversed control here: instead of Shifty calling the schedule function, we want to call the processTweens from the game engine at regular interval (if needed for better results, we can take care of calling it at regular intervals instead of a variable time delta) (but not sure it will change anything).

Thanks for any precision :) The integration of Shifty is being done here: 4ian/GDevelop#922

@jeremyckahn
Copy link
Owner

jeremyckahn commented Feb 23, 2019

Hi @4ian, thanks for opening this issue! I've seen GDevelop before; it's a seriously cool project and I'd love to help with integrating Shifty into it! I can see that there's already quite a bit of disussion and exploration on your end and I haven't had a chance to get fully caught up, but this is a really cool use case for Shifty so you can count on me to help update it to suit your needs. :)

Can we instantiate multiple instances of Shifty?

Yep! Shifty is designed to support an arbitrary number of instances so this shouldn't be an issue. processTweens is an internal implementation detail that processes all tweens globally in order to optimize for performance, but it maintains individual tween state. So, tween A could be paused while tween B is running. If that's not working then that's a bug in Shifty and I will prioritize a fix.

Can we get rid of the default requestAnimationFrame, and instead call manually the function processTweens? I've seen it's exported in tweenable.js, but not part of the public API?

processTweens wasn't exported because it's fairly low-level and wasn't sure if there was a use case for it. I opted to keep the public API smaller until a need to expose it arised. I suppose that this is that need. :) It should be a pretty simple change and I'd be happy to make it, but I'd prefer the public name to be something like tick so it's a bit more generic semantically.

I've seen Tweenable.setScheduleFunction and Tweenable.now too. Tweenable.now could be redefined to return the time of the game engine (so that tweens are following any time scaling/pausing applied to the game), but I'm not sure about setScheduleFunction. We would need an inversed control here: instead of Shifty calling the schedule function, we want to call the processTweens from the game engine at regular interval (if needed for better results, we can take care of calling it at regular intervals instead of a variable time delta) (but not sure it will change anything).

This is interesting. To meet your needs, it's possible that scheduleUpdate will need to be changed, but I don't think that will be necessary. I think you'll be able to achieve what you need by doing tweenableInstance.setScheduleFunction(() => {}) Tweenable.setScheduleFunction(() => {}) and externally calling the public tick/processTweens function as discussed above.

There may be a bit more that I haven't considered, but based on what you've described in this issue I think the minimal change needed in Shifty is to expose the internal processTweens functionality. From there, GDevelop can set Tweenable.now and call tweenableInstance.setScheduleFunction(() => {}) for each instance Tweenable.setScheduleFunction(() => {}). If you'd prefer to statically define the schedule function (similar to Tweenable.now) I'm open to adding an API for that, but I'd like to know that it would helpful for you before doing that.

Does this all sound correct to you? I should have some time this weekend to at least start on some of these changes if you can confirm that I've got this right!


EDIT: I forgot that I deprecated the setScheduleFunction instance method; it is much better to use the static alternative going forward.

@4ian
Copy link
Author

4ian commented Feb 23, 2019 via email

@jeremyckahn
Copy link
Owner

I'm only unsure about pausing all tweens/starting them again between scenes (that why I was hoping to be able to create multiple instances, so that I can just not tick the Shifty instance of scene that are not running). Hooking in the scene start/pause to pause tweens could be possible but a bit more complex as we would need to store the list of tweens to stop and restart.

I think this is going to be an issue with the proposed approach. While overriding the control of tick scheduling shouldn't cause any issues, the tweens' individual timelines won't automatically "shift" to account for what seems like global "pause" logic. In other words, Shifty tweens are scheduled around real time. If a tween's duration is 1000 milliseconds, Shifty will stop() the tween after 1000 real milliseconds unless pause or seek is called to shift the end time appropriately. I expect that "globally resumed" tweens that were previously in the background will snap to their to positions, which I think is what you are concerned about.

Off the top of my head, I can think of two solutions to this problem:

  1. Use a Tweenable instance to orchestrate calls to interpolate in the step handler. interpolate is not time-based so it will not snap to the end as I described above, and the orchestrating Tweenable instance can easily be controlled with standard play/pause/etc. playback control methods.

  2. Consider using Rekapi to manage your animations. Rekapi is closely related to Shifty and actually uses it as the underlying state processing engine. It's a more comprehensive solution to do what I described in my first suggestion and should be able to manage multiple timelines in a more straightforward way.

I may be overcomplicating this a bit. In any case, I'll start on exposing the processTweens functionality publicly so we can at least get some traction on this. I'll create a new branch and let you know when it's ready for you to try out in GDevelop!

@jeremyckahn
Copy link
Owner

@4ian This is now in the feature/109/expose-process-tweens branch. Please pull it down and give it a try! I thought about it some more and actually chose to stick with the processTweens name. tick implies some sort of timing management that isn't there and update seems too vague in the contexts I expect this will be used in.

Let me know how this goes for you! If it helps to solve your problem I will merge this branch and make a new release of Shifty.

@jeremyckahn
Copy link
Owner

Also please keep in mind that you will need to run npm run build in order to generate the built files in dist.

@jeremyckahn
Copy link
Owner

I thought about this all some more and I think I might have slightly misinterpreted one of your earlier questions:

Can we instantiate multiple instances of Shifty? A "level" is represented by a "scene" in GDevelop, and multiple scenes can be existing at the same time.

While Shifty does support an arbitrary number of Tweenable instances, the tween processing logic works as a singleton. While taking control of the scheduling of updates will help with 4ian/GDevelop#922, what's missing in Shifty is any concept of scene management. This is all more or less what we've discussed so far.

I'm thinking that it might be appropriate for Shifty to provide proper scene management utilities. Something similar to Rekapi, but lighter weight and generic enough to address the needs of GDevelop and similar use cases. Basically, something small to control the playback states of groups of tweens, perhaps like so:

import { tween, Scene } from 'shifty';

const sceneA = new Scene(
  // Tween 1
  tween({
    from: { x: 0 },
    to: { x: 10 },
    duration: 1000
  }),
  // Tween 2
  tween({
    from: { x: 50 },
    to: { x: 100 },
    delay: 500,
    duration: 1000
  })
);

const sceneB = new Scene(
  // Tween 3
  tween({
    from: { x: 25 },
    to: { x: 100 },
    duration: 1000
  }),
  // Tween 4
  tween({
    from: { x: 100 },
    to: { x: 50 },
    duration: 1500
  })
);

// Pauses tweens 1 and 2 while 3 and 4 play
sceneA.pause();

I don't want this to get into sophisticated sequence management, as that's exactly what Rekapi exists to do. But I think GDevelop is going to need something like this to manage paused background tweens.

I'm willing to develop something like this in Shifty to be used by GDevelop, but this is a relatively large new feature so it would take me a bit of time implement properly. I'm also open to PRs if you need this sooner rather than later and would prefer to have Shifty provide this functionality instead of doing it in GDevelop.

@4ian Would you prefer that Shifty provide scene management, or have GDevelop take care of it?

@4ian
Copy link
Author

4ian commented Feb 24, 2019

This is now in the feature/109/expose-process-tweens branch. Please pull it down and give it a try! I thought about it some more and actually chose to stick with the processTweens name. tick implies some sort of timing management that isn't there and update seems too vague in the contexts I expect this will be used in.

Awesome! I'll pull this and build the dist files :)
Thanks for working on this so quick. I had little time this week-end but I'll give it a go this week.

But I think GDevelop is going to need something like this to manage paused background tweens.

Yes you right, I think we will need this.

Basically, something small to control the playback states of groups of tweens, perhaps like so:

That would work I guess, as we could have all tweens in a scene being played while other are paused.

Maybe I'm totally wrong, but what about allowing to have more than one Shifty instance? Not sure what kind of refactoring would be needed or if it's even a good idea but that would provide a perfect, 100% bullet proof isolation of tweens: in a scene A, we would create a Shifty instance, creating tweens on it, then in scene B we would have a totally separate Shifty instance, and tweens created using this instance. GDevelop will call processTweens and will provide the time manually using Tweenable.now, so all of the tweens will be 100% isolated. (not sure about the implication of this again in terms of refactoring or performance)

But if you think scenes are doable, we can go with this solution.
We'll use the singleton instance, as now, and use the new concept of "scenes" to create tweens (or attach them to a scene, whatever the API is). Then, we'll have to make sure to call pause/play when the GDevelop scene is paused/restarted :)

Note that if you think everything is becoming too complicated, maybe we can switch to Rekapi. But having a lightweight tween library like Shifty is really cool, especially if we can guarantee the isolation of tweens :)

@jeremyckahn
Copy link
Owner

Maybe I'm totally wrong, but what about allowing to have more than one Shifty instance? Not sure what kind of refactoring would be needed or if it's even a good idea but that would provide a perfect, 100% bullet proof isolation of tweens: in a scene A, we would create a Shifty instance, creating tweens on it, then in scene B we would have a totally separate Shifty instance, and tweens created using this instance.

My concern with the "instancing" approach is that it would require significant modification of the batched processing system that was introduced in the 2.5.0 release. That release introduced a singular, internal linked list of all tweens that is processed upon every tick.

I don't think that supporting what you are describing would introduce significant performance penalties, but it would complicate the code quite a bit — more than I feel comfortable with for such a specific use case. While no-compromises performance is a top priority of this library, another high priority is for the API and code to be relatively readable and understandable... least of which for myself as the maintainer! 😅

To give this decision a bit of context, Greensock provides the absolute best performance of any animation library, and in performance stress tests it outperforms Shifty (though the difference is insignificant in even the most contrived scenarios). However, the code itself is (necessarily) a bit obtuse, and I don't want Shifty's code to become unapproachable or unmaintainable in the name of supporting uncommon use cases.

Assuming for a moment that we change Shifty to support multiple instances of the internal linked list, actually taking advantage of it reveals a bit of a leaky abstraction. It would necessitate a fairly deep understanding of the internal mechanics of the library which I feel should be avoided by users. It seems that the need here is to manage different sets of tweens independently, and I feel that a higher-level "Scene" API is the most straightforward way to achieve that.

Note that if you think everything is becoming too complicated, maybe we can switch to Rekapi. But having a lightweight tween library like Shifty is really cool, especially if we can guarantee the isolation of tweens :)

I think that Rekapi would do everything you're looking for without the need for modification or API contortions. However, I think it would also necessitate quite a bit of rework to 4ian/GDevelop#922, so I don't want just throw this work over the wall to you and @Wend1go et al. Also Rekapi can't reach quite the level of animation performance as Shifty because it is quite a bit more abstracted, and performance is critical in a game engine. I feel that a lightweight "Scene" API on top of Shifty is the most pragmatic solution here, either in GDevelop or provided by Shifty.

I'm going to open a new issue for Shifty to start speccing out how this might work. Please don't feel compelled to wait for or use my solution, but I think this is at least interesting enough to start exploring. I'll leave this issue open until you give the 👍 to the changes in the feature/109/expose-process-tweens branch and it is released.

@jeremyckahn jeremyckahn mentioned this issue Feb 25, 2019
@jeremyckahn
Copy link
Owner

This seems to be working, and this is a pretty innocuous change, so I just merged and released it: https://github.com/jeremyckahn/shifty/releases/tag/v2.7.0

New API docs: https://jeremyckahn.github.io/shifty/doc/shifty.html#.processTweens

@4ian I hope this helps! Let me know if GDevelop needs anything else. 🙂

@4ian
Copy link
Author

4ian commented Feb 26, 2019

Huge thanks for adding this so quickly. Can confirm this works properly in GDevelop!

My concern with the "instancing" approach is that it would require significant modification of the batched processing system that was introduced in the 2.5.0 release. That release introduced a singular, internal linked list of all tweens that is processed upon every tick.

I see, I've see this linked list while initially browsing the source code, so I understand this might get tricky.

Assuming for a moment that we change Shifty to support multiple instances of the internal linked list, actually taking advantage of it reveals a bit of a leaky abstraction

Yeah sure, I was more hoping for something that would "just" be a factory function for the whole module, so that no global are used and we can freely create new "Shifty.js". But understand this might be an issue in terms of simplicity/performance (and would only be useful in the case you're manually calling processTweens, like GDevelop is doing).

Scene API draft looks good so far! Will comment more on the other issue :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants