4ian / GDevelop

🎮 Open-source, cross-platform 2D/3D/multiplayer game engine designed for everyone.
https://gdevelop.io
Other
10.83k stars 849 forks source link

Scene Timers: No longer automatically initialized since 5.0.126 #3499

Closed AtomicCrocStudios closed 2 years ago

AtomicCrocStudios commented 2 years ago

Describe the bug

I was working on a project today and noticed the new timers were not firing for me. Thinking I may have somehow screwed up something simple I tried creating a test project to just update a text object with the current timestamp every second, once again it didn't work. image

After looking at the RAW mode of the scene debugged, I could see that the timer wasn't even being initialized. image

I tried initializing the timer manually at the start of the scene and it worked. image

I am hoping this is not the intended behaviour of this update as I believe it has the ability to break existing projects once they're updated to the latest version with users assuming that timers still automatically initialize.

I understand it's bad practice to not manually initialize them, however it always felt unnecessary to run an action that was already handled by the engine on startup.

Happy to hear any feedback and thanks for expanding upon the functionality D8H :)

4ian commented 2 years ago

Timer are indeed not initialised automatically anymore, you need to call the "reset the timer" whenever you want to start it.

But this should not break any project because the old condition is still working as before. This being said, should we add a parameter to "start the timer if it was not started" or should be just adapt the documentation and our habits to start a timer explicitly? In theory, being explicit is better than implicit here.

cc @D8H :)

AtomicCrocStudios commented 2 years ago

Timer are indeed not initialised automatically anymore, you need to call the "reset the timer" whenever you want to start it.

But this should not break any project because the old condition is still working as before. This being said, should we add a parameter to "start the timer if it was not started" or should be just adapt the documentation and our habits to start a timer explicitly? In theory, being explicit is better than implicit here.

cc @D8H :)

Thanks for the reply 4ian. I think having people manually create the timers is perfectly fine and gives more control to the user in the long run (they may not want to initialize at the beginning of the scene).

Rather then adding the new option, maybe just some info text on the condition?

image

Silver-Streak commented 2 years ago

I still think a parameter on timer conditions (or actions) could be very useful, have it default to "initialize at start of scene: yes" for backwards compatibility, and then users still have the option to not do so if they don't want to.

Bouh commented 2 years ago

I still think a parameter on timer conditions (or actions) could be very useful, have it default to "initialize at start of scene: yes" for backwards compatibility, and then users still have the option to not do so if they don't want to.

I did not see that change in the changelog, maybe it deserves a warning for the ancient users? I agree it could be more friendly to have a boolean button on the timer condition/action to "initialize at start of scene: yes/no"

4ian commented 2 years ago

Agree, I will make a warning on the changelog (my bad, this change was added just before the release and I did not think enough it could be confusing).

I'll also add a warning on the conditions: image

I agree it could be more friendly to have a boolean button on the timer condition/action to "initialize at start of scene: yes/no"

Well that was the problem of the previous version: it did not initialize the timer at the start of the scene. Instead, it started the timer the first time a condition referring to the timer was executed. So this created confusion (everyone thinks the timer was started at the start of the scene, but no, it was just when the condition was executed). This being said, we could do again this behavior with a toggle "Automatically start the timer if was never started", so that we would both have the previous behavior (not sure if it should be the default or not), and the new operators.

Bouh commented 2 years ago

"Automatically start the timer if was never started"

It's more confusing. Since we don't know when it starts, you said it yourself before it was on execution (I didn't know that before you said it here). And now it's not even automatic anymore, we have to set the action ourselves to start a timer.

Or I didn't understand anything and in this case, it's more worrying :')

What should be explained is in the boolean field in the description is: "True = Automatically started the timer when the scene start / False = The timer start when the condition is checked" And False is set by default to keep the compatibility like before ever.

With this boolean, we don't need to add an action manually at the beginning of the scene to start a timer, which is very wordy. It's for me a bad and terrible choice made here to force the user to make it manually, and it could broke all the older projects.

D8H commented 2 years ago

This being said, we could do again this behavior with a toggle "Automatically start the timer if was never started", so that we would both have the previous behavior (not sure if it should be the default or not), and the new operators.

I'm utterly against it.

Silver-Streak commented 2 years ago

This being said, we could do again this behavior with a toggle "Automatically start the timer if was never started", so that we would both have the previous behavior (not sure if it should be the default or not), and the new operators.

I'm utterly against it.

  • In every language I know, a condition with a side effect is an anti-pattern.
  • With the <= operator, starting the timer means that if the condition is checked again right below it will be true. <= is useful to check if something happens in less than N seconds, so it makes no senses to start it in this configuration.
  • Why kill the cat?

@D8H Did you mean to reply to a different part of the text? Your bullets don't seem to match up to the text you're replying to, or I can't quite parse it within the context.

4ian commented 2 years ago

No I think D8H answer is right. Just to make things clear:

1) This is not breaking older games. This is a new condition, and the old one still works but is hidden.

2) The old condition was confusing. Timers were started when the first condition referring to them was executed (and NOT at the beginning of the scene).

3) This being said, with this condition, we now have to always use the action to start a timer. It's not the end of the world. BUT it can take time for people to adapt to this. So the question is :

Silver-Streak commented 2 years ago

Thanks for the clarification!

  • Is this ok?

It could be, but we have seen a decent amount of feedback from users who are very confused as this wasn't listed in any release notes they (or I) can find.

  • If not, do we want to:

    • Reproduce the old behavior (D8H is against this).
    • Do something were we analyze the timers used and start them at the beginning of the scene (but it's unlikely we can do that, because timers name are expressions and can be anything).

I still think a toggle is probably the right way for compatibility and user flexibility going forward, explicitly because it can help users reduce the amount of extra events in their event sheet to reproduce behavior they are used to for years now.

ClementPasteau commented 2 years ago

I've updated the release notes to highlight the change of behavior for the Timers. I agree that we should have planned this better!

D8H commented 2 years ago

Let's think of this with game theory:

New condition

Case A: users are used to the old way

Dangerousness: 1/4 hour loss Risk: 75% at release and probably 25% on newcomers

Case B: user have to add a start timer action to start a timer

Dangerousness: 1 minute loss Risk: 100%

Old condition (or switch)

Case C: users hut a bug caused by a timer starting at the condition check

Dangerousness: 1 day + support time + might discourage users + games released with bugs Risk: 10% (those 10% are likely to be working on a big games that make GDevelop shines which make the dangerousness even more dangerous)

Bouh commented 2 years ago

It's certainly easier to add a warning message to tell the user to add a timer himself but it's an avoidable warning...

By adding a boolean on the new condition (enable automatically the timer or not at the beginning of the scene) we let him know that something has changed because he has never seen the new button True/False on a timer in the past, this way the new condition becomes easier to use and to understand.

I can't imagine saying to people with lot of timers in their games, now you've to add a new action for all your timers. I agree it give more controls but for the user who use this new timers it's a new thing to do more. And to finish, the toggle would give more freedom, is I enable the timers by myself where I want in the eventsheet, or the engine do it for me at the beginning.

D8H commented 2 years ago

It's certainly easier to add a warning message to tell the user to add a timer himself but it's an avoidable warning...

By adding a boolean on the new condition (enable automatically the timer or not at the beginning of the scene) we let him know that something has changed because he has never seen the new button True/False on a timer in the past, this way the new condition becomes easier to use and to understand.

By adding a boolean on the new condition, we let users doing something they don't understand and will backfire at them later just to save few clicks and we allow some configuration that can't work < and Yes. Last week, I wouldn't be able to help a user with a timer that doesn't start at the beginning because I didn't know this could happen and I've being using GDevelop for 1 year. Why bother trying to explain something very complicated and error prone when we can just let users create an action when they want things to be done. I mean this is the core concept of GDevelop.

It seems this is short term vs long term. This is a new condition, let's make it right, clean slate.

Users have bad habits, habits can change and imho for the best.

Silver-Streak commented 2 years ago

Side note: Assuming the old event isn't restored with a toggle, All of the timer examples probably need to be pulled from the main examples list until someone updates them. They'll work, but won't match what the users can create anymore and they won't understand why.

https://editor.gdevelop-app.com/?project=https://resources.gdevelop-app.com/examples/count-down-timer/count-down-timer.json

https://editor.gdevelop-app.com/?project=https://resources.gdevelop-app.com/examples/objects-timers/objects-timers.json

(Count down timer may work since it's resetting at the start of the scene)

4ian commented 2 years ago

That's true we'll definitely need to update events and the wiki as soon as possible if we keep this this way (which seems to be the "good way to go", despite the cost in terms of habits to change - and because resetting all timers at the beginning of the scene can't work because we don't know which timers will be started in the future).

tristanbob commented 2 years ago
  1. This is not breaking older games. This is a new condition, and the old one still works but is hidden.

How does this deprecation work? If I open an old game, and then add a new timer condition, which version does it use? I think it is important for the entire project to use the same version of the condition. So if a project is using the previous timer condition, adding new timer condition will also use that same version.

As far as upgrading a project to use the new timer condition, can this be done like the Z-order change was done? I thought it was executed well, but I don't know how it affected others.

  1. The old condition was confusing. Timers were started when the first condition referring to them was executed (and NOT at the beginning of the scene).

Yes, it confused me too (I understand it now). Checking a condition should never change anything (like Davy said).

  1. This being said, with this condition, we now have to always use the action to start a timer. It's not the end of the world. BUT it can take time for people to adapt to this. So the question is :
  • Is this ok?

Yes, I like the information and wording you used in the screenshot.

As a side note, we need to change the text in "Reset a scene timer" sentence to match the name of the action "Start (or reset) a scene timer". It doesn't make sense to only see a "reset" action when the user wants to "start" the timer. I can submit this wording change if no one gets to it.

D8H commented 2 years ago

How does this deprecation work? If I open an old game, and then add a new timer condition, which version does it use? I think it is important for the entire project to use the same version of the condition. So if a project is using the previous timer condition, adding new timer condition will also use that same version.

As far as upgrading a project to use the new timer condition, can this be done like the Z-order change was done? I thought it was executed well, but I don't know how it affected others.

Using a setting to unhide some deprecated functions is an interesting idea.

For this case, old and new conditions can coexist in an old project without any issues. Users only needs to start manually the new timers they're adding (and they can start every old one too if they want). It only takes one old condition to start it automatically.

ddabrahim commented 2 years ago
  1. The old condition was confusing. Timers were started when the first condition referring to them was executed (and NOT at the beginning of the scene).

What makes you think that it was confusing? It was always obvious to me timers were created the first time a condition was referring to them.

A warning as AtomicCrocStudios suggested would be not sufficient and to make sure it is explained in the documentation?

What you are discussing here only add unnecessary bloat in my opinion which personally I dislike. As a beginner, I really liked the simplicity of GDevelop and the fact it does initialise everything for me and does not force me in to a frame even if it's not practical it was an amazing experience for someone with no prior knowledge. Now that I am more experienced I dislike bloat even more, I love straight forward, simple solutions. I extremely dislike when certain options are hidden behind menus within an other menu that's hidden within an extension that need to be download/enabled using an other hidden menu somewhere and need to click 5 times and navigate between a dozen menus to get something otherwise very simple done and something you may need frequently.

In case, it is really not ok to just display a warning and explain in the documentations that timers won't start until a condition refer to them and you really want to force people to start timers manually, personally I would opt for explicitly "Start the timer" using a single event to avoid unnecessary bloat and confusion with parameters hidden somewhere and to keep it simple to follow in the events.

However you decide, going forward I would like to ask the team to consider:

Good luck and thank you 👍

RagnarRandom commented 2 years ago

i agree with the notion that there should be both a beta version and a stable version. i can't think of any open source software other than gdevelop that doesn't do this.

Bouh commented 2 years ago

As this issue has become a thread with everyone's opinion, and as some changes have been made to rename the start timer action, the examples have been updated with the new timers, add a warning in the patch note, add a pinned post on the forum, and a pinned post in the announcement discord channel I'm converting this thread from issue to the discussion.