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

Remove irritating Interface > Dialogs category from Editor Settings #14550

Closed
RayKoopa opened this issue Dec 11, 2017 · 16 comments · Fixed by #91073
Closed

Remove irritating Interface > Dialogs category from Editor Settings #14550

RayKoopa opened this issue Dec 11, 2017 · 16 comments · Fixed by #91073

Comments

@RayKoopa
Copy link
Contributor

As discussed in #14549, I wanted to prevent the interface/dialogs/* settings to be visible in the Editor Settings because:

  • Configuring saved window bounds for a handful of selected windows via a Rect2 property editor is needlessly overcomplicated
  • The user can indirectly modify this editor settings by actually moving / resizing those windows
  • Modifying editor_settings_bounds doesn't work anyway as it is overridden when the Editor Settings window is closed
  • The category is irritating and might suggest more usefully configurable settings here

image

I originally implemented hiding it in the PR linked above by skipping over the category with a hardcoded check for the name, but that's not a good solution as pointed out by @akien-mga .

How would I be able to hide such "indirectly useful but directly useless" settings from the Editor Settings without the need for hardcoding category names?

@bojidar-bg
Copy link
Contributor

Shouldn't we keep this information separate from the settings, for example alongside layouts?

@RayKoopa
Copy link
Contributor Author

Interesting idea, I like it so far.

@groud
Copy link
Member

groud commented Dec 11, 2017

If those should not be configurable, just "hardcode" the values in the editor code.
Like in settings_dialog_config.cpp: line 98, 99 and 144.

@RayKoopa
Copy link
Contributor Author

I don't have such file, can you link to it? We previously discussed not to hardcode hiding them as linked above.

@groud
Copy link
Member

groud commented Dec 11, 2017

Sorry I meant settings_config_dialog.cpp.
Yeah, instead of hiding them, you just don't make them configurable. You can make the value an attribute of the class for example.

@RayKoopa
Copy link
Contributor Author

Then the whole category is useless, just displaying read-only values. Nah, that's still ugly IMHO. Get rid of them in the settings, there's already too much in them.

@groud
Copy link
Member

groud commented Dec 11, 2017

Yeah, if you don't make them configurable, it means removing them from the settings... The category will be removed automatically once you removed all the properties inside it.

@RayKoopa
Copy link
Contributor Author

RayKoopa commented Dec 11, 2017

Oh nice, I didn't know that it works like that exactly. What property usage do I have to specify then?

@groud
Copy link
Member

groud commented Dec 11, 2017

I don't get what you mean. For example at line 144, instead of:
EditorSettings::get_singleton()->set("interface/dialogs/editor_settings_bounds", get_rect());
you can define an attribute called "editor_bounds" of the class maybe. And put instead:
editor_bounds = get_rect()

But I'm not sure it could work like that, as the property may need to be global.

@RayKoopa
Copy link
Contributor Author

And how are the window bounds stored and restored from settings if it's just a temporary field of the class?

@groud
Copy link
Member

groud commented Dec 11, 2017

Reading the code, it might work by just putting an underscore before the property name.
So that EditorSettings::get_singleton()->set("interface/dialogs/editor_settings_bounds", get_rect()); becomes EditorSettings::get_singleton()->set("interface/dialogs/_editor_settings_bounds", get_rect());

@RayKoopa
Copy link
Contributor Author

No, it still appears then.

@27thLiz
Copy link
Contributor

27thLiz commented Dec 11, 2017

You could also put them in the project metadata file. (EditorSettings::set_project_metadata())

@RayKoopa
Copy link
Contributor Author

It's saved per-project then, if I got that right, I'm not sure if that makes sense. Per-layout would make more sense, aka @bojidar-bg 's solution

@groud
Copy link
Member

groud commented Dec 11, 2017

Yeah, probably that @bojidar-bg's proposal makes sense too. I'm allright with that.

@Calinou
Copy link
Member

Calinou commented Jun 18, 2020

This appears to have been mostly resolved in the master branch. Only one dialogs/ setting will be saved now when you close the 2D polygon UV editor:

void Polygon2DEditor::_uv_edit_popup_hide() {
EditorSettings::get_singleton()->set("interface/dialogs/uv_editor_bounds", Rect2(uv_edit->get_position(), uv_edit->get_size()));
_cancel_editing();
}

Once this last use has been removed and replaced with the new system (I don't know what it is), this issue can be closed.

Edit: Still valid as of 4.0.alpha11.

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

Successfully merging a pull request may close this issue.

6 participants