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

Minor refactoring & new twist #5

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

TheSeems
Copy link

Refactored twist menu & config to achieve less duplication
Added new twist which basically teleports players on the top block at their coordinates (to avoid spending 30+ minutes mining) - only in the overworld

int random = ThreadLocalRandom.current().nextInt(plugin.getCfg().seeds.size());
long seed = plugin.getCfg().seeds.get(random);
this.seed = seed;
this.seed = ThreadLocalRandom.current().nextLong();
Copy link
Collaborator

Choose a reason for hiding this comment

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

This removes the seed pre-selection system

Copy link
Author

Choose a reason for hiding this comment

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

I guess you are looking at one particular commit. It was later reverted.

@@ -368,7 +368,7 @@ public void prepareForSpectate() {
assert player != null;
player.setInvisible(true);
reset(player, false);
player.setGameMode(GameMode.SURVIVAL);
player.setGameMode(GameMode.SPECTATOR);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This removes clickable items in hotbar as a spectator

gp.updateScoreboard();
gp.getCompassTracker().updateCompass();

if (gp.isSpectating()) {
player.setGameMode(GameMode.SPECTATOR);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unnecessary at best, or at worst, admins can't change gamemode

@@ -89,7 +89,7 @@ public void openMenu(Player player) {
gui.setItem(i, plugin.getItemizer().FILL_ITEM);
}

gui.setItem(53, plugin.getItemizer().createItem(plugin.getCfg().mainMenuStoreMaterial, 1, Util.c(plugin.getCfg().mainMenuStoreDisplayname), plugin.getCfg().mainMenuStoreLore));
gui.setItem(53, plugin.getItemizer().FILL_ITEM);
Copy link
Collaborator

@TechnicallyCoded TechnicallyCoded Jul 17, 2022

Choose a reason for hiding this comment

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

Removes the main store menu item??


public static IconConfig load(String prefix, FileConfiguration fileConfiguration) {
return new IconConfig(
fileConfiguration.getString(prefix + "-material"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not use prefix + ".material" as to use sub sections in the config?

Copy link
Author

Choose a reason for hiding this comment

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

I guess i can do that but it will require config migrations (current keys for icons as far as i remember are ...-material, ...-displayname and ...-lore)

Copy link
Collaborator

@TechnicallyCoded TechnicallyCoded left a comment

Choose a reason for hiding this comment

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

Everything in the before last commit revert changes from the first it seems?
Advancement reset would be good but needs to be part of a util class or method, there is no point for the repetition.

@TechnicallyCoded
Copy link
Collaborator

The name of the twist "get high" could also be revisited. I don't think it represents what you really want.

@TheSeems
Copy link
Author

I agree, but i can't think of a proper name for that twist

@TheSeems
Copy link
Author

TheSeems commented Jul 17, 2022

I guess advancement reset should be a separate toggleable feature so this PR is not about that.
Initially, I played freely with the codebase to adapt it for the local community but I cleaned it up later so that this PR could be consistent.

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