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

Customizable Language Selection Panel #2254

Merged
merged 5 commits into from
Jan 4, 2024
Merged

Conversation

BONNe
Copy link
Member

@BONNe BONNe commented Jan 2, 2024

This implements customizable Language Selection Panel. By default, panel is generated in /plugins/bentobox/panels folder, however, if GameModeAddon has a specific panel defined in their folder, then that panel is taken.

This implements customizable Language Selection Panel. By default, panel is generated in `/plugins/bentobox/panels` folder, however, if GameModeAddon has a specific panel defined in their folder, then that panel is taken.
@BONNe BONNe marked this pull request as draft January 2, 2024 13:56
@BONNe BONNe marked this pull request as ready for review January 4, 2024 06:52
@BONNe BONNe changed the title WIP: Customizable Language Selection Panel Customizable Language Selection Panel Jan 4, 2024
@BONNe
Copy link
Member Author

BONNe commented Jan 4, 2024

Hi @tastybento

This is a fully implemented customizable language selection panel for users. I also implemented the unit test, but I am unsure if it tests everything. But for me, it passed.

@BONNe BONNe requested a review from tastybento January 4, 2024 06:54
Copy link

sonarqubecloud bot commented Jan 4, 2024

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

12 New issues
0 Security Hotspots
37.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@@ -463,6 +463,10 @@ public boolean loadSettings() {
getPluginLoader().disablePlugin(this);
return false;
}

log("Saving default panels...");
this.saveResource("panels/language_panel.yml", false);
Copy link
Member

Choose a reason for hiding this comment

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

This begs a question - if we used false as the second parameter then if this panel ever needs to be updated by us, e.g., new code, then we will have to have admins remove the old panel yml file when updating. If however, we use true, then we will overwrite the file and ensure that the latest file is used with the latest code. Do you think there's ever going to be a need for admins to adjust these panels? And even if they did, due to code updates, their old yml may not work with the new release, so it'd be better to overwrite it. What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

I do not think it is necessary to overwrite them. Otherwise how admins will be able to customize their menus if we overwrite them always.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, good point. The only thing then going forward is that instructions for upgrading will need to include directions to delete the old panel file (or move it) if the change is affected by it. During my development of the team GUI I've had a number of times I forgot to delete it and wondered why I was getting weird operations.

Copy link
Member

@tastybento tastybento 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

@tastybento tastybento merged commit 3be034b into develop Jan 4, 2024
2 checks passed
@tastybento tastybento deleted the language_selection_panel branch January 4, 2024 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants