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

Particles with "one shot" enabled gets "emitting" turned off #17903

Open
MelvinWM opened this Issue Apr 1, 2018 · 8 comments

Comments

Projects
None yet
2 participants
@MelvinWM
Copy link
Contributor

MelvinWM commented Apr 1, 2018

Godot version:
098c7ba4f9c49b472b9417819144378081996874
(current master)

Issue description:

When using the node Particles2D or Particles, and having both properties "Emitting" and "One Shot" enabled, if saving the configuration somehow (for instance by moving the focus away from the node), "Emitting" becomes false in the editor, and does not play when the game is run either. See also the GIF below:

particles_emitting_one_shot_bug_2018-04-01_2

Proposed fix
I think the regression happened in 1f609b7. I do not know what the purpose of that commit was; it seems a pure regression AFAICT. That said, I do not know Godot or its particle system very well, so this might be wrong. I have tested a simple reversion, and that seemed to work: In the editor, "emitting" was kept and not discarded, and "one shot" worked when running the game.

@MelvinWM MelvinWM referenced this issue Apr 1, 2018

Open

[TRACKER] Particles issues #10239

21 of 30 tasks complete
@mateusak

This comment has been minimized.

Copy link
Contributor

mateusak commented Apr 1, 2018

The point of the mentioned commit is to facilitate particles emission control by runtime scripts.
It should not function in editor, though, as one may want the particles to be one shot but start automatically at instantiation.

Fix:
While in editor, do not automatically disable emitting after emission completion when one shot is enabled.

@MelvinWM

This comment has been minimized.

Copy link
Contributor Author

MelvinWM commented Apr 1, 2018

Ah, that makes sense, if the getter for "Emitting" only returned for a field in the Particles(2D) in the scene layer, it would not be possible to get the correct value once emission was over, since "Emitting" is only disabled in the "driver" with "One Shot".

Can "TOOLS_ENABLED" be used to check whether one is in the editor, or should something else be used?

Also, I believe it would be better if it still stopped emitting in the editor, since if I understand you correctly, that would make "One Shot"=true have emission continue while in the editor, which I believe is not desirable. Instead, I think a solution might be while in the editor to keep and use a field for "Emitting" in the scene layer, and when not in the editor ignore that field in the scene layer like how it currently works.

@mateusak

This comment has been minimized.

Copy link
Contributor

mateusak commented Apr 1, 2018

Not quite right. Before that commit, when you enabled emitting with one shot on, it would spawn the particles once, leaving emitting 'on', which made it confusing to know in which state the Particles2D is currently in.
If you apply the fix I mentioned, it will visually function identically as it is now in the editor, with the only difference the preferred default state will be saved, and will function correctly at runtime.
Other potentially less confusing fix is to add a new option 'emit on instantiation'.

@MelvinWM

This comment has been minimized.

Copy link
Contributor Author

MelvinWM commented Apr 1, 2018

I think I understand better now. But does that then mean that for "option 1", if "Emitting" is off, whether "One Shot" is on or off, and whether "Emitting" was turned off by the game developer or by "One Shot" running, it should still emit when running the game? Making "Emitting" in effect more of an editor-only property, at least from the game developer's POV?

Edit: Sorry, I am wrong, it would be from the game developer's POV while in the editor, not while using scripting.

@ghost ghost added enhancement topic:editor labels Apr 3, 2018

@MelvinWM

This comment has been minimized.

Copy link
Contributor Author

MelvinWM commented Apr 3, 2018

@mateusak I have thought about it some more and looked into it, and I think your second proposal regarding an option like "Emit on Instantiation" would be best. That approach would also enable having the same overall behaviour as AudioStreamPlayer2D, which has both "Playing" and "Autoplay". Maybe call it "Autoemit"?

The only issue I can see at the moment is backwards compatibility regarding "Emitting", since if "Emitting" is on (and "One Shot" is off), the generated game has particles emitting for that node, while if "Emitting" is false, the generated game does not have particles emitting for that node. With this change, that would instead be controlled by "Emit on Instantiation"/"Autoemit". How important is backwards compatibility in this case?

Also, assuming that the desired functionality is determined, would it be OK if I tried to implement it, test it and make a pull request?

@MelvinWM

This comment has been minimized.

Copy link
Contributor Author

MelvinWM commented Apr 4, 2018

@mateusak Maybe a way to have backwards compatibility would be to let the proposed property "Autoemit" be disabled by default, and if it is disabled, let it default to the current behaviour. Or maybe let "Autoemit" have 3 states, "Enabled", "Disabled" and "Unset", and let it default to the current behaviour if in "Unset".

@mateusak

This comment has been minimized.

Copy link
Contributor

mateusak commented Apr 4, 2018

@MelvinWM
This is actually an issue with the primitive inspector Godot has. There should be an start emission
and stop emission function and a loop boolean instead of one shot, then an autoemit would fit. But since Godot's inspector doesn't support buttons, they cant trigger those functions, so they went with a boolean and are now trying to buttonmize it. So now we have this strange situation where nothing fits quite right.
In this situation, the best solution is to hide the autoemit property until one shot gets enabled, but then again, Godot doesn't support this either.

Godot offers a lot, but its usability is seriously compromised by the antiquated text editor and inspector. If you want to fix the root of this problem, I suggest working on the inspector.

@MelvinWM

This comment has been minimized.

Copy link
Contributor Author

MelvinWM commented Apr 8, 2018

@mateusak
Regarding which design and UX Godot ought to have regarding the Inspector and the like, I know far too little of Godot to be able to really offer any real insights.

I think one of the really neat things about Godot, at least from what I have seen so far, is that it is really easy and intuitive to both modify and see the results of modifications just in the editor. For instance, you can have multiple particle effects running and see the results directly in the editor without having to run the game. Or play an animation, complete with sound and particle effects running. How well it works I do not know, but it seems to work at least OK, the basic "Platformer 2D" example project does showcase some of these features.

As far as I can tell, there are two main needs and goals related to the Inspector and UI elements like the bottom tabs named "Audio" and "Animation":

  • The user needs to be able to view and modify nodes and properties (and other abstractions like signals) for the purpose of changing the behaviour of the game when it is run.
  • The user benefits from being able to see directly in the editor the effects of changes to nodes and properties.

From looking at the code, it seems that the implementation that deals with these two needs are somewhat intermingled. In particular, I believe that the editor state for nodes overlaps or is shared with the node and property definitions used for generating games to be run. I fear that such a mingling can make the code more complicated and bug-prone than it has to be, though I do not at the moment see a good alternative. That said, I might well be wrong about that, and this is after all just games and game tools, so the quality requirements are not that high (higher for game tools than for games of course), and other concerns such as performance, flexibility, maintainability and functionality matter as well.

Assuming it might be that I am not off or way off regarding this superficial analysis with overlapping or shared state between the editor and the node/property definitions, do you happen to have any good intuition regarding whether there are few or many bugs that might stem directly or indirectly from this? If you do not at the moment have any good intuition regarding this, I could try to look into it. Of course, it may just be that I am still very new to Godot, and that this is a non-issue or has been discussed and planned for, etc.

There is also the aspect of supporting live editing, which I do not have experience with or considered regarding this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.