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

Organize existing code for editor plugins #90975

Merged
merged 1 commit into from
Apr 29, 2024

Conversation

aaronfranke
Copy link
Member

@aaronfranke aaronfranke commented Apr 21, 2024

I was annoyed that the base class for editor plugins wasn't in the editor/plugins/ folder, nor were the files for changing editor plugin settings or editing project plugins. This PR moves these files into this folder.

Aside from moving files, this PR also modifies PluginConfigDialog to have the logic for selecting a language and script name organized together, instead of being mixed with the name/folder/description/author/version logic. This was the main motivation for this PR, I already have another PR lined up which needs this code nicely organized.

This PR has one behavior change: In the current master, when creating a new plugin, it will create a new script. However, if you change the file name of a plugin's script, it will not create a new one with the new name. Now it does. Note that it will only create a file if the file is missing, so it will never overwrite existing files.

@KoBeWi
Copy link
Member

KoBeWi commented Apr 22, 2024

if you change the file name of a plugin's script, it will not create a new one with the new name. Now it does.

I think it should first try to rename/move existing file.

if (!_edit_mode) {
void PluginConfigDialog::_create_script_for_plugin(const String &p_plugin_path, Ref<ConfigFile> p_config_file, int p_script_lang_index) {
ScriptLanguage *language = ScriptServer::get_language(p_script_lang_index);
ERR_FAIL_COND(language == nullptr);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ERR_FAIL_COND(language == nullptr);
ERR_FAIL_NULL(language);

@aaronfranke
Copy link
Member Author

@KoBeWi I don't know the correct way to rename a script in the editor. If I just rename it using DirAccess, the script editor is not informed of the change, so the edited script has the wrong file name still.

@KoBeWi
Copy link
Member

KoBeWi commented Apr 23, 2024

You can call update_file() in EditorFileSystem, or in worst case, just do rescan().

@aaronfranke
Copy link
Member Author

@KoBeWi That does not inform the script editor of the change.

@KoBeWi
Copy link
Member

KoBeWi commented Apr 23, 2024

And if you use this?
KoBeWi@7c165a1
(FileSystemDock::get_singleton()->rename_file())

@aaronfranke
Copy link
Member Author

@KoBeWi That still does not work, and it gives a new error too:

Screenshot 2024-04-24 at 9 39 36 PM

@KoBeWi
Copy link
Member

KoBeWi commented Apr 25, 2024

I was going to look into it, but mixing the behavior change with such enormous refactor makes it unreadable. You should split this into 2 commit.

@aaronfranke aaronfranke force-pushed the plugin-org branch 2 times, most recently from 96301e0 to 626bc91 Compare April 25, 2024 22:56
@aaronfranke
Copy link
Member Author

aaronfranke commented Apr 26, 2024

@KoBeWi I updated the PR to remove the behavior change. The other commit can be done in another PR.

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.

Plugin config dialog changes look fine.

@akien-mga akien-mga modified the milestones: 4.x, 4.3 Apr 29, 2024
@akien-mga akien-mga merged commit 13fbd42 into godotengine:master Apr 29, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

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.

4 participants