Add screens to Rainbow #9
Conversation
eclipseisoffline
left a comment
eclipseisoffline
left a comment
There was a problem hiding this comment.
Initial batch - will probably be more reviews once these are taken care of. I'll also be sure to play around with the screens to test the user interaction.
| context.getSource().sendFeedback(Component.translatable("commands.rainbow.pack_created", name)); | ||
|
|
||
| PackManagerUtils.startPack(name, packManager, context.getSource().getClient()); | ||
|
|
There was a problem hiding this comment.
Unnecessary whitespace.
| } | ||
|
|
||
| public CustomItemProvider getItemProvider() { | ||
| if (this.itemProvider == null) return NoItemProvider.INSTANCE; |
There was a problem hiding this comment.
As this is a (relatively) new project there's still a chance to have a consistent codestyle without having to reformat everything.
Until there is an organisation wide standard for code style, please fully expand if statements and the likes, as this is most consistent with other code.
|
|
||
| public void tick(Minecraft minecraft) { | ||
| // Don't tick when this screen is open, the user might be toggling settings | ||
| if (minecraft.screen instanceof ManagePackScreen) return; |
There was a problem hiding this comment.
Make this check if the game is paused (Minecraft#isPaused) as well as if the open screen should pause the game (Screen#isPauseScreen), then the instanceof check here should no longer be necessary.
|
|
||
| import java.util.concurrent.atomic.AtomicReference; | ||
|
|
||
| @Mixin(AbstractContainerScreen.class) |
There was a problem hiding this comment.
Would it be possible to remove the mixin and use Fabric's screen API instead? (see net.fabricmc.fabric.api.client.screen.v1)
| private final PackMapper mapper; | ||
|
|
||
| @Nullable | ||
| private EditBox PACK_NAME_EDIT_BOX; |
There was a problem hiding this comment.
Should be packNameEditBox.
| import org.slf4j.Logger; | ||
|
|
||
| public class RainbowClient implements ClientModInitializer { | ||
| public static final Logger LOGGER = LogUtils.getLogger(); |
There was a problem hiding this comment.
Can replace this with Rainbow.LOGGER.
| "SplashRendererAccessor" | ||
| ], | ||
| "injectors": { | ||
| "defaultRequire": 1 |
There was a problem hiding this comment.
Indents need fixing here.
| private final PackManager packManager = new PackManager(); | ||
| private final PackMapper packMapper = new PackMapper(packManager); | ||
| private static final PackManager packManager = new PackManager(); | ||
| private static final PackMapper packMapper = new PackMapper(packManager); |
There was a problem hiding this comment.
These are constants, so PACK_MANAGER and PACK_MAPPER.
| public static PackManager getPackManager() { | ||
| return packManager; | ||
| } | ||
| public static PackMapper getPackMapper() { |
There was a problem hiding this comment.
Add a newline before this method.
| List<ItemStack> items; | ||
|
|
||
| if (minecraft.screen instanceof AbstractContainerScreen<?> abstractContainerScreen) { | ||
| items = abstractContainerScreen.getMenu().getItems(); | ||
| } else { | ||
| items = new ArrayList<>(minecraft.player.getInventory().getNonEquipmentItems()); | ||
| items.add(minecraft.player.getInventory().getItem(36)); // Boots | ||
| items.add(minecraft.player.getInventory().getItem(37)); // Leggings | ||
| items.add(minecraft.player.getInventory().getItem(38)); // Chestplate | ||
| items.add(minecraft.player.getInventory().getItem(39)); // Helmet | ||
| items.add(minecraft.player.getInventory().getItem(40)); // Offhand | ||
| } |
There was a problem hiding this comment.
| List<ItemStack> items; | |
| if (minecraft.screen instanceof AbstractContainerScreen<?> abstractContainerScreen) { | |
| items = abstractContainerScreen.getMenu().getItems(); | |
| } else { | |
| items = new ArrayList<>(minecraft.player.getInventory().getNonEquipmentItems()); | |
| items.add(minecraft.player.getInventory().getItem(36)); // Boots | |
| items.add(minecraft.player.getInventory().getItem(37)); // Leggings | |
| items.add(minecraft.player.getInventory().getItem(38)); // Chestplate | |
| items.add(minecraft.player.getInventory().getItem(39)); // Helmet | |
| items.add(minecraft.player.getInventory().getItem(40)); // Offhand | |
| } | |
| List<ItemStack> items = new ArrayList<>(minecraft.player.inventoryMenu.getItems()); | |
| if (minecraft.player.hasContainerOpen()) { | |
| items.addAll(minecraft.player.containerMenu.getItems()) | |
| } |
2 participants
Add GUI screens to Rainbow in order to make it much more user-friendly (imo anyway).
This PR also shuffles round translations and changes them to word them better for the user, adds a dummy
CustomItemProviderfor null safety (uses a null value in the backend anyway).