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

Fix Embedded Game Size #101807

Merged
merged 1 commit into from
Jan 22, 2025
Merged

Conversation

Hilderin
Copy link
Contributor

@Hilderin Hilderin commented Jan 19, 2025

This PR fixes multiple issues related to the game window size when embedding is enabled.

The main challenge was to find a way to support a mode where the game size stretches to fit the Game Workspace when developing games where the window size is not crucial, while also supporting a fixed size when it is important.

The solution I suggest is to add a new fixed-size mode. In this mode, even if the Game Workspace is larger, the window size defined in the project settings will be used. This mode is now the default.

Additionally, the Floating Window did not respect the project and editor settings for window size and placement. This PR modifies the Floating Window to have an initial size that accounts for the controls and margins of the Game Workspace. As a result, upon starting, the Game Window should always align with the project/editor settings. A side effect of this modification is that the position and size of the Floating Window will be reset with each run.

I was unsure what to do with minimized, maximized, and fullscreen modes. For now, embedding is not disabled when these modes are enabled. The game will start as usual.

image

image

image

image

image

@Hilderin Hilderin requested a review from a team as a code owner January 19, 2025 18:26
@Hilderin Hilderin added this to the 4.4 milestone Jan 19, 2025
@Hilderin Hilderin force-pushed the fix-embedded-game-size branch 2 times, most recently from e2c2f64 to 75bfcd6 Compare January 19, 2025 18:32
Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

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

  • Embedded window scaling seems to be working fine.
  • Floating window placement is placed inconsistently. Note: I think this mode should be redone in exactly opposite way of the current implementation, toolbar embedded into a game process or floating near it, this will make it much more usable.
  • I'm not sure about scaling mode icons, stretch mode icon is a bit different from other icons and keep ratio icon is not intuitive, maybe something like this:
    404685592-f8740dfe-6dc8-42a2-bf37-8a2d1a2f6bf8 or 404685592-f8740dfe-6dc8-42a2-bf37-8a2d1a2f6bf8

@Hilderin
Copy link
Contributor Author

Note: I think this mode should be redone in exactly opposite way of the current implementation, toolbar embedded into a game process or floating near it, this will make it much more usable.

I find myself agreeing more and more with this idea every time I work with the floating window. It would offer several benefits, such as compatibility with macOS, support for multiple game instances, and preserving the standard game window layout. After careful consideration, I see three possible approaches:

1. Transforming the current floating window into a small toolbar window that moves with the game window:

Pros:

  • Requires fewer modifications to implement.

Cons:

  • Does not work in single-window mode.
  • Incompatible with fullscreen/maximized modes.
  • Not supported on web or Android.
  • May cause synchronization issues with window placement and order.
  • Difficult to manage multiple instances, as currently only one floating window instance exists.

2. Using a floating toolbar window near the game window that belongs to the game process:

Pros:

  • Likely easier to maintain
  • Could enable interesting new features.
  • Should work on macOS.

Cons:

  • If implemented using standard nodes, it would require adding nodes to the game at runtime—something that hasn't been done before.
  • The toolbar code from GameViewPlugin cannot be reused as-is and would need to be rewritten.
  • Does not work in single-window mode.
  • Incompatible with fullscreen/maximized modes.
  • Not supported on web or Android.
  • Potential issues with window placement, e.g., windows cannot be moved outside the screen area in X11.

3. Embedding the toolbar as a panel overlay within the game window:

Pros:

  • Could be implemented relatively easily without adding nodes to the game if done similarly to the button in this [PR](Implement extend to title for Windows and Linux #96310), but it's a little more complex.
  • Could enable interesting new features.
  • Compatible with all platforms.
  • Supports games with multiple windows.
  • Should work in single-window mode.

Cons:

  • If standard nodes are used, it would require runtime additions to the game, which hasn't been done before.
  • Managing the toolbar's size and scaling could be challenging, as it would render inside the game window based on its resolution and size.
  • A mechanism to show/hide the toolbar, such as a keyboard shortcut, would be necessary.

Anyway, I think that should be a separate PR and probably a proposal.

@Hilderin Hilderin force-pushed the fix-embedded-game-size branch 2 times, most recently from d21b81e to 1c43920 Compare January 21, 2025 00:21
@Hilderin
Copy link
Contributor Author

I changed the icons following your suggestion:
image

Comment on lines 473 to 479
case EMBED_NOT_AVAILABLE_MINIMIZED:
state_label->set_text(TTR("Game embedding not available when the game starts minimized."));
break;
case EMBED_NOT_AVAILABLE_MAXIMIZED:
state_label->set_text(TTR("Game embedding not available when the game starts maximized."));
break;
case EMBED_NOT_AVAILABLE_FULLSCREEN:
state_label->set_text(TTR("Game embedding not available when the game starts in fullscreen."));
break;
Copy link
Member

@Calinou Calinou Jan 22, 2025

Choose a reason for hiding this comment

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

These 3 modes could have a string appended at the end to suggest the use of feature tags to override the window mode when running the project from the editor:

Consider overriding the window mode project setting with the editor feature tag to Windowed to use game embedding while leaving the exported project intact.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added your suggested text to the messages. I also added some margin to the right and left sides to prevent the text from touching the edges.

I did not know we could do that in Godot!! Thanks!

image

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally, it works as expected. Code looks good to me. Thanks a lot for tackling those issues; combined with #101809, this fixes most of the usability regressions I noticed on my end when using game window embedding.

Note that it's still possible for the viewport size to be smaller than the project settings when using the mode that uses the viewport size from the project settings, but I think this makes sense for situations where you use a high base viewport size to cater to 4K displays (but are not using a 4K display yourself).

image

@Hilderin Hilderin force-pushed the fix-embedded-game-size branch from 1c43920 to 26d2e15 Compare January 22, 2025 00:50
@Hilderin Hilderin force-pushed the fix-embedded-game-size branch from 26d2e15 to ae0b7ff Compare January 22, 2025 01:18
@Repiteo Repiteo merged commit fef1bc6 into godotengine:master Jan 22, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Jan 22, 2025

Thanks!

@SilverWolveGames
Copy link

Thank you @Hilderin !

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