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

Add "Game" editor for better runtime debugging #97257

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

YeldhamDev
Copy link
Member

@YeldhamDev YeldhamDev commented Sep 20, 2024

Implements several aspects of godotengine/godot-proposals#7213.

2024-09-20.19-19-42.mp4

Features Implemented

  • Selection of 2D/3D nodes at runtime when the game is suspended (see below), this includes anything that has collisions or some sort of visual representation. When a node is selected, a selection box will appear around it and it will be opened in the inspector.
  • Suspend mode. This is a middle ground between in-game pausing and the editor's break feature. When this mode is enabled, it allows node selection, and frame skipping.
  • Next frame. This allows the game to run one frame at the time when the it's suspended.
  • 2D/3D camera override. The camera manipulation can come from within the game, or the 2D and 3D editors.

Things Missing

  • Window embedding. This requires low-level OS-specific code, that's above my league.

Limitations

  • For now, selection only works in the root window. This is not a hard limitation, and selection in subwindows could be implemented as well.
  • No editor theming for the selection boxes and popup list, as well as no shortcut redefinition for camera controls, since we can't access those values at runtime. A possible workaround would be to pass those settings via the debugger.

Compatibility Break

  • A minor one is unavoidable to add this new editor to the feature profile.

Sponsored By: 🐺 Lone Wolf Technology / 🍀 W4 Games.

@YeldhamDev YeldhamDev added this to the 4.x milestone Sep 20, 2024
@YeldhamDev YeldhamDev requested review from a team as code owners September 20, 2024 22:34
@YeldhamDev YeldhamDev marked this pull request as draft September 20, 2024 23:05
@YeldhamDev YeldhamDev force-pushed the guess_godot_is_unity_after_all branch 2 times, most recently from f27cb5e to aa9fd47 Compare September 21, 2024 03:01
@JoNax97
Copy link
Contributor

JoNax97 commented Sep 21, 2024

This is excellent!

One small detail I think would be neat is to freeze the time parameter in shaders while the game is suspended, so that the effects don't continue animating and the 'next frame' button allows for shader debugging

@YeldhamDev
Copy link
Member Author

@JoNax97 Done!

@RobProductions
Copy link
Contributor

Finally, this is incredible work and I'm really excited to see this merged! Thank you so much! A few questions:

  1. I can't tell from the video but does the click selection also work to highlight the tree item in the "remote" tab? I see it selects the remote note in the inspector which is great but it would also be cool to quickly navigate a complex hierarchy by selecting stuff, which would quickly unfold the parent items. I believe I've seen a plugin do this recently.
  2. Would it possible to separate the "suspend" mode from the click selection so that users can click on things while the game is not paused? Like maybe with a separate toggle to activate a selection mode instead of being tied to suspend? I think this was something mentioned in the original proposal with the "selection mode", and being able to click on (and eventually edit I presume) nodes while the game is running could save a lot of time for realtime tech demos (like cloth physics demos as an example) that would otherwise have to implement their own selection tools. I'm also imagining a debug camera being the next step here and that might fit better in a selection mode rather than suspend mode because you could take a look at something more closely while its in motion/changing instead of having to unpause and repause and find that exact spot again.
  3. Finally, and this may be a big ask, but is there potential for users to extend the functionality of these new runtime tools with custom code? For example, if there was never a PR for changing camera in pause mode, a user might want to code their own camera system that hooks into the new suspend code. But since I noticed that the can_process method now checks for suspend status, no user code can be run while the game is paused. This makes sense as a quick way to halt scripts but it would be neat to allow some way to customize how the tools work, though I'm not exactly sure how. On that note, it would also be great to get a signal for suspend and unsuspend to coincide with the notifications, that way at least user code can run when the mode starts and stops :)

I don't think all of that necessarily needs to go in this PR but just curious about the future of this feature. Regardless, I really appreciate what you have so far!

@YeldhamDev
Copy link
Member Author

@RobProductions

  1. Yes! And clicking an item in the remote tree also highlights the node in the game as well.
  2. Internally we settled to make selection only available when suspended, as the inputs of clicking around would clash with in-game code.
  3. This sort of stuff would need to have its own proposal opened for discussion, as this is beyond the scope of this PR.

@m4gr3d
Copy link
Contributor

m4gr3d commented Sep 22, 2024

Great work!

I'm curious how this would behave for an XR project.. would it be possible to disable the Game tab for an XR project but keep the functionality shown in the video?
cc @BastiaanOlij

Also I'm curious if this would work with the Android editor. Have you tested this PR with the Android editor?

@rayzorite

This comment was marked as off-topic.

@Kathenae

This comment was marked as off-topic.

@Faless
Copy link
Collaborator

Faless commented Sep 22, 2024

3. Finally, and this may be a big ask, but is there potential for users to extend the functionality of these new runtime tools with custom code? For example, if there was never a PR for changing camera in pause mode, a user might want to code their own camera system that hooks into the new suspend code. But since I noticed that the can_process method now checks for suspend status, no user code can be run while the game is paused.

@RobProductions I haven't checked the code yet, but like this PR is doing, you can have your own code evaluated outside of process (even during debug breaks), by registering a capture in EngineDebugger and implementing a EditorDebuggerPlugin to communicate with it.

@adevinwild

This comment was marked as off-topic.

editor/editor_node.cpp Outdated Show resolved Hide resolved
@Chaosus
Copy link
Member

Chaosus commented Sep 22, 2024

Nice work, I would like to see more controls for the interacted objects (in Unity you can move, rotate, scale in this mode?), maybe some new buttons needs to be introduced for this.

@KoBeWi
Copy link
Member

KoBeWi commented Sep 22, 2024

  • Sometimes the suspend button is stuck disabled:
fbq8107lZ3.mp4

This happens after running the project a couple of times and requires restarting editor to fix.

EDIT:
Seems to happen when you F8 to close the game. Looking at the implementation, maybe session counter is broken.

EDIT2:
Yep, F8 closes the session twice and makes the counter go negative permanently.

  • The selection popup appears at wrong position. Seems to be relative to "Game" position.
godot.windows.editor.dev.x86_64_BOfYg60Jin.mp4
  • It would be useful to have in-game shortcut for next frame. Clicking a button isn't always convenient.

Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and can confirm it works, with some of the issues mentioned above like the selection box positioning, but otherwise works great!

@jaydensipe
Copy link
Contributor

jaydensipe commented Oct 22, 2024

After that, Engine::get_singleton()->is_in_physics_frame() always returns true inside of the function for me.

@YeldhamDev @GustJc @MBCX Yo, could you guys try this MRP if you've got time, I am seeing the same behavior when trying on 4.3 stable & 4.4 with GDScript. I promise I'll put this to rest lol, but wanted to see if is_in_physics_frame() returns true always for you guys.

test-mrp.zip

image

image

@YeldhamDev
Copy link
Member Author

@jaydensipe
image

@MBCX
Copy link
Contributor

MBCX commented Oct 22, 2024

@jaydensipe Tested 4.3-stable and 4.4.dev trunk from this branch. Both always return true for me.

4.3-stable

Godot v4.3.stable - Windows 10.0.22631 - Vulkan (Forward+) - dedicated Radeon RX 580 Series (Advanced Micro Devices, Inc.; 31.0.21921.1000) - Intel(R) Core(TM) i5-9600K CPU @ 3.70GHz (6 Threads)
image

v4.4.dev.gh-97257 [2ee6e45]
Godot v4.4.dev (2ee6e45) - Windows 10.0.22631 - Multi-window, 2 monitors - Vulkan (Forward+) - dedicated Radeon RX 580 Series (Advanced Micro Devices, Inc.; 31.0.21921.1000) - Intel(R) Core(TM) i5-9600K CPU @ 3.70GHz (6 threads)

image

@YeldhamDev YeldhamDev force-pushed the guess_godot_is_unity_after_all branch from 2535b8f to 9672068 Compare October 22, 2024 17:46
@jaydensipe
Copy link
Contributor

jaydensipe commented Oct 22, 2024

@YeldhamDev @MBCX Alright I believe I figured it out, or at least the cause. Do you guys have 60hz monitors?

If you do, when running the project with V-Sync ENABLED under project settings causes the project FPS and physics rate to be equal (60). So if I test the "Game" editor with 60hz monitor and V-Sync ENABLED, I don't crash, but if you disable V-Sync and try to test, it SHOULD crash for you guys now.

image

test-mrp-with-fps-stuff-and-vsync-disabled.zip

Running with 240hz on my monitor:
image

Running with 60hz on my monitor (V-Sync ON):
image

Running with 60hz on my monitor (V-Sync DISABLED):
image

So I've also read a bit about the deferred stuff here which might have some insight (https://www.reddit.com/r/godot/comments/uk7kzz/call_deferred_is_for_idle_frames_but_why_not_also/).

@MBCX
Copy link
Contributor

MBCX commented Oct 22, 2024

Yup @jaydensipe, it does crash when vsync is disabled

image

@jaydensipe
Copy link
Contributor

Yup @jaydensipe, it does crash when vsync is disabled

image

Ok, thank god I am not crazy, thank you guys for testing that!

@YeldhamDev I think in node_3d_editor_plugin.cpp line 8078, that is a workaround someone used, but you might be able to find a better way.
image

@MBCX
Copy link
Contributor

MBCX commented Oct 22, 2024

I tested that on my second AOC display which is 60 Hz, I tested again now on my first Samsung monitor (which according to Windows is a 59.94 Hz monitor) and I did get a true case while testing, interesting... 🤔

image

In 90% of cases though it crashes. And because is 59.94 I guess the engine rounds Physics Frames to 60.

@jaydensipe
Copy link
Contributor

I tested that on my second AOC display which is 60 Hz, I tested again now on my first Samsung monitor (which according to Windows is a 59.94 Hz monitor) and I did get a true case while testing, interesting... 🤔

image

In 90% of cases though it crashes. And because is 59.94 I guess the engine rounds Physics Frames to 60.

Yep, I found the same, sometimes you get lucky and happen to be in the physics frame.

@ydeltastar
Copy link
Contributor

Alright I believe I figured it out, or at least the cause. Do you guys have 60hz monitors?

I have a 160hz and another 60hz monitor. It only returns true when the window in the 60hz but when I move it to the 160hz monitor, it randomly returns false or true when I click.

@GustJc
Copy link
Contributor

GustJc commented Oct 22, 2024

After that, Engine::get_singleton()->is_in_physics_frame() always returns true inside of the function for me.

@YeldhamDev @GustJc @MBCX Yo, could you guys try this MRP if you've got time, I am seeing the same behavior when trying on 4.3 stable & 4.4 with GDScript. I promise I'll put this to rest lol, but wanted to see if is_in_physics_frame() returns true always for you guys.

Oh, that's interesting....

Normally it is true here..... changing vsync inside godot doesn't seem to affect at first.
image

But that was because I've set vsync turned ON inside my nvidea configuration panel.
Disabling vsync in the nvidea control panel.... now I've got lots of FALSE's.... and even got a crash if clicking too much.
image

@YeldhamDev YeldhamDev force-pushed the guess_godot_is_unity_after_all branch from 9672068 to 708f818 Compare October 22, 2024 22:16
@YeldhamDev
Copy link
Member Author

@jaydensipe Alright, one more time, please test it.

@jaydensipe
Copy link
Contributor

jaydensipe commented Oct 22, 2024

@jaydensipe Alright, one more time, please test it.

@YeldhamDev Hmm... still seems to be crashing. Did you change any code though, or did you wanna try the RenderingDevice change in the above branch compare https://github.com/godotengine/godot/compare/9672068184828d8e948d5b28b12f367f4d617739..708f818f31574a99cc388f5c3958b477a11db356?

@YeldhamDev YeldhamDev force-pushed the guess_godot_is_unity_after_all branch from 708f818 to 2f206f5 Compare October 22, 2024 22:55
@YeldhamDev
Copy link
Member Author

My changes were unstaged when rebasing from the master branch and I didn't notice when commiting. 🫠

For real now, test it.

@jaydensipe
Copy link
Contributor

My changes were unstaged when rebasing from the master branch and I didn't notice when commiting. 🫠

For real now, test it.

@YeldhamDev All good lolol

Looks like... WE ARE GOOD! Great job man. 🎊
As for a last little nitpick, which is whatever, is the camera override seems to be moving in physics_process now instead of process, which makes it feel a little sluggish (when again using a higher refresh monitor), but other than that everything is completely good now.

Screen.Recording.2024-10-22.191251.mp4

@YeldhamDev YeldhamDev force-pushed the guess_godot_is_unity_after_all branch from 2f206f5 to b7b2540 Compare October 23, 2024 00:53
@YeldhamDev
Copy link
Member Author

@jaydensipe Done!

@jaydensipe
Copy link
Contributor

@jaydensipe Done!

Works great now!

@clayjohn clayjohn requested a review from m4gr3d October 23, 2024 21:07
@arkology
Copy link
Contributor

I thought about some edge case of nodes selection in running project and assumption was confirmed:

Running project will be closed (crashed?) after trying to select from popup menu a node which doesn't exist anymore. This does not cause the editor to crash.

Test project: new_project.zip

Select Sprite2D when it is inside the scene tree to bring up the popup menu (you may have to click in the center of the screen since Node2D and Camera2D are in the center) and wait until Sprite2D is freed. After Sprite2D is freed, try to select it from the popup menu list. The project will close.

@YeldhamDev YeldhamDev force-pushed the guess_godot_is_unity_after_all branch from b7b2540 to 882d607 Compare October 24, 2024 17:20
@YeldhamDev
Copy link
Member Author

@arkology Again, good catch. Fixed.

Copy link
Contributor

@m4gr3d m4gr3d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran into a couple issues when testing the latest updates to this PR:

  • Show list of selectable nodes at position clicked is still broken on Android; the popup is not interactable and prevent any further input
1000059422.mp4
  • 2D node selection breaks when manipulating camera from the editor. This occurs both on Android and Windows
1000059444.mp4

@YeldhamDev YeldhamDev force-pushed the guess_godot_is_unity_after_all branch from 882d607 to e75d88f Compare October 25, 2024 18:53
@YeldhamDev
Copy link
Member Author

@m4gr3d

2D node selection breaks when manipulating camera from the editor. This occurs both on Android and Windows

Fixed.

Show list of selectable nodes at position clicked is still broken on Android; the popup is not interactable and prevent any further input

As I said before, this sounds like a popup bug on Android, unrelated to the PR itself. I can't even test for a workaround on my end, as I don't own a tablet.

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

Successfully merging this pull request may close these issues.

Add a keyboard shortcut to perform frame-by-frame stepping when running a project from the editor