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 multi window code and shader editors (GSOC'22 Project) #62378

Merged
merged 1 commit into from
May 10, 2023

Conversation

trollodel
Copy link
Contributor

@trollodel trollodel commented Jun 24, 2022

This is the main PR for my GSOC'22 project (link to the project description).
Old commit history with expanded commits: https://github.com/trollodel/godot/tree/gsoc_2022_multiwindow_full_history

The project consists in the following major features:

  • Script Editor undocking.
  • Shader Editor undocking.
  • Window layout saves.

You can read more about it in the GSOC'22 progress report: https://godotengine.org/article/gsoc-2022-progress-report-1

The PR is feature complete, further enhancement will be done in a separate PR, after the merge of this one, which will happen after the 4.0 release as marked by the milestone.

Screenshots/Videos
Peek.24-06-2022.12-06.mp4
Peek.15-07-2022.12-42.mp4
Peek.15-07-2022.13-39.mp4
Peek.19-07-2022.15-06.mp4

The screen selector in the Script Editor (two 1920x1080 screen)

The screen selector with different screen sizes (one 1920x1080 screen and one 1280x960 screen, 4:3 aspect ratio)

Everything undocked!

Part of godotengine/godot-proposals#28.

Happy testing!

Supersedes #62963.

Keywords for easy searching: detach script, detachable, detached, pop out, popping out, other window, separate window, undock, undocked, undockable, floating

@jcostello
Copy link
Contributor

Looking good so far. As I mention in the other PR discussion: F1 is not working inside the code window while its focus

@Chaosus Chaosus added this to the 4.0 milestone Jun 24, 2022
@Calinou Calinou modified the milestones: 4.0, 4.1 Jun 24, 2022
@trollodel
Copy link
Contributor Author

@jcostello sorry for the late response, but now global shortcuts (like F1 or Ctrl+S) should work in the script window too.

@jcostello
Copy link
Contributor

@jcostello sorry for the late response, but now global shortcuts (like F1 or Ctrl+S) should work in the script window too.

Nice. Im out til the weekend. I will test it then :)

@trollodel trollodel force-pushed the gsoc_2022_multiwindow branch 4 times, most recently from 9994552 to 3622226 Compare July 19, 2022 14:03
@trollodel trollodel force-pushed the gsoc_2022_multiwindow branch 4 times, most recently from b6c7fbe to 7296303 Compare August 3, 2022 14:54
@trollodel trollodel force-pushed the gsoc_2022_multiwindow branch 2 times, most recently from 35443f7 to d73a49e Compare August 18, 2022 08:44
@trollodel
Copy link
Contributor Author

trollodel commented Aug 18, 2022

The PR is now feature complete for what I have planned. The only thing remaining to do are a bunch of editor settings to customize the behavior.
Some commits done here will be extracted in dedicated PRs as they are independent, but required, changes.
To indicate the progresses, I'll mark the PR as ready for review, even if it's not ready to be merged (starting from the verbose commit history).

Again, Happy testing!

@trollodel trollodel marked this pull request as ready for review August 18, 2022 18:30
@trollodel trollodel requested review from a team as code owners August 18, 2022 18:30
@YeldhamDev
Copy link
Member

YeldhamDev commented Aug 18, 2022

Could you please rebase this to the latest master commit if possible? If it's too much for the quantity of commits in the PR then ignore this.

@YeldhamDev
Copy link
Member

Icons in the "Search Help" dialog appear to be broken:
image

@trollodel
Copy link
Contributor Author

trollodel commented May 5, 2023

I found out why the screen popup doesn't do anything, but I don't know what causes it .-. The buttons do not emit the pressed signal. It's as if they don't receive clicks. It might be because you're abusing OptionButton's popup for that instead of a custom one, but not sure why would it make difference, so maybe it's not that. But the buttons are unclickable. Which is weird, because you said it works for you.

I did some testing regarding the screen popup issue. The buttons don't receive pressed mouse events. Seems like a problem exclusive to PopupMenu. I guess using MenuButton this way is wrong. So instead ScreenSelect should be a regular Button with a custom PopupPanel.

Ok now it uses regular Button and Popup, It should work for you now.

@KoBeWi

This comment was marked as resolved.

@trollodel
Copy link
Contributor Author

trollodel commented May 7, 2023

The popup is now functional. The only problem is that it doesn't close once clicked upside_down_face

I can't reproduce it on Linux. Just in case, I forcefully hide the popup when the button is pressed.

The F1 shortcut is still broken. It needs to be fixed.

After the rebase, the F1 shortcuts now work when docked and undocked.

Also there are some conflicts, so the PR needs rebase.

Done.

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

KoBeWi commented May 9, 2023

I found an edge case. When you make docks floating and restart editor in single mode, the docks will be unusable:
image
Not critical though.

Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

All obvious/important issues are fixed.

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Looks good to me from a cursory code check, and I tested the functionality a bit and it seems to work well.

I think it's time for a merge and get it tested more widely :)

For the record, I'll release 4.1-dev2 tomorrow but I already built it from a commit prior to merging this, so this will be in an official build in 4.1-dev3.

@akien-mga
Copy link
Member

I noticed this warning in the terminal output (and in the editor toaster) while opening a project:

Warning emitted when opening any project: "Attempting to access theme items too early; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED"

I previously ran into it with #76509 which was fixed by #76513. This PR might introduce another case of using the theme too early.

As a side note, while testing this PR I saved a project with 2 floating docks, which were properly remembered across sessions. Then I reopened that same project in a Godot build from the master branch without this PR, and the floating docks were still floating and functional. Pretty cool feature that works retroactively :P

editor/window_wrapper.cpp Outdated Show resolved Hide resolved
@trollodel
Copy link
Contributor Author

For the record, I'll release 4.1-dev2 tomorrow but I already built it from a commit prior to merging this, so this will be in an official build in 4.1-dev3.

So sad, users won't have this absolutely desired feature tomorrow, they would wait for other 2 weeks (sarcasm).

As a side note, while testing this PR I saved a project with 2 floating docks, which were properly remembered across sessions. Then I reopened that same project in a Godot build from the master branch without this PR, and the floating docks were still floating and functional. Pretty cool feature that works retroactively :P

I tested myself and I don't really understand why it works. I guess the docks were placed in the default slot as a failsafe.

@AThousandShips
Copy link
Member

AThousandShips commented May 9, 2023

CI should be fixed by #76885, will have to be rebased when it is merged

@akien-mga akien-mga merged commit 31fc7a8 into godotengine:master May 10, 2023
@akien-mga
Copy link
Member

Congrats on landing this amazing feature! 🎉
Thanks for your patience with the review and release processes :)

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.