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

Hide unsupported features in the web editor #49540

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

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Jun 12, 2021

The game camera override button is disabled with a tooltip explaining why, but other menu options are hidden entirely to prevent them from being listed in the list of available editor shortcuts.

The Scan button is hidden as it serves no purpose in the web project manager. There is no way to import multiple project folders at once to the virtual HTML5 filesystem.

This pull request can be remade on the 3.x branch once we agree on its design.

@Calinou Calinou requested a review from a team as a code owner June 12, 2021 17:38
@Calinou Calinou added this to the 4.0 milestone Jun 12, 2021
@Calinou Calinou changed the title Hide supported features in the web editor Hide unsupported features in the web editor Jun 12, 2021
Copy link
Member

@fire fire left a comment

Choose a reason for hiding this comment

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

I didn't see any removals that were obviously bad. We can always remove more later.

@Calinou Calinou force-pushed the web-editor-hide-unsupported-features branch from b5041b8 to 4c959ad Compare June 13, 2021 16:55
@akien-mga akien-mga requested a review from Faless June 20, 2021 10:41
@akien-mga
Copy link
Member

That makes sense, though I'm not fond of having so many hardcoded defines for a given platform in editor code. WDYT @Faless?

@Faless
Copy link
Collaborator

Faless commented Jun 23, 2021

That makes sense, though I'm not fond of having so many hardcoded defines for a given platform in editor code. WDYT @Faless?

I feel the same... Can we monkey patch the options via the editor tool javascript plugin? Maybe it's worse...

In any case, we don't need the ifndefs in the _menu_options callbacks if we hide the UI. That should at least remove some pre-processor noise.

@Calinou
Copy link
Member Author

Calinou commented Jun 23, 2021

In any case, we don't need the ifndefs in the _menu_options callbacks if we hide the UI. That should at least remove some pre-processor noise.

I needed those to avoid a crash or mismatched option indexes when the JavaScript platform was active.

@Calinou Calinou force-pushed the web-editor-hide-unsupported-features branch 2 times, most recently from 88ef5a5 to 1378f90 Compare March 18, 2022 01:57
@fire
Copy link
Member

fire commented Mar 19, 2022

@akien-mga Was there a reason this was limboed?

@akien-mga
Copy link
Member

See the above discussion, neither @Faless nor myself really like the way this is done.

@akien-mga
Copy link
Member

akien-mga commented Sep 16, 2022

In the end we went for the same hacky approach for the Android editor to fill the editor code with #ifdef ANDROID_ENABLED, so I guess there's no reason not to allow doing the same for the Web editor.

Should be rebased / retested so we can review further (though the Web editor still doesn't start right now so you might want to wait for that).

Moving to 4.x milestone as not urgent, but if it's ready to merge during the 4.0 beta phase it should be good to go.

The game camera override button is disabled with a tooltip
explaining why, but other menu options are hidden entirely
to prevent them from being listed in the list of available
editor shortcuts.

The Scan button is hidden as it serves no purpose in the web project
manager. There is no way to import multiple project folders at once
to the virtual HTML5 filesystem.
@Calinou Calinou force-pushed the web-editor-hide-unsupported-features branch from 1378f90 to 9b2c05d Compare September 29, 2022 16:07
@@ -6831,20 +6843,28 @@ EditorNode::EditorNode() {
settings_menu->add_submenu_item(TTR("Editor Layout"), "Layouts");
settings_menu->add_separator();

#if !defined(WEB_ENABLED) && !defined(ANDROID_ENABLED)
Copy link
Member

Choose a reason for hiding this comment

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

This wasn't excluded on Android. Did you check if it works?

Comment on lines +2707 to +2711
#ifdef WEB_ENABLED
// Scan function is not relevant on the web editor, as there is no way
// to import multiple projects at once into the virtual HTML5 filesystem.
scan_btn->hide();
#endif
Copy link
Member

Choose a reason for hiding this comment

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

Did you test importing a zip with multiple projects in subfolders?

Copy link
Member Author

@Calinou Calinou Sep 30, 2022

Choose a reason for hiding this comment

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

I've just tried this in 4.0.beta2. It only imports the first project that is found in the ZIP. It'd be nice to implement support for importing multiple projects in a single ZIP (either at top-level or within a subfolder).

projects_at_top_level.zip
projects_in_subfolder.zip

@akien-mga akien-mga marked this pull request as draft January 4, 2024 16:15
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.

None yet

5 participants