Datapack confirmation (#14)

- Added an optional confirmation screen for all datapack changes. This includes an option to exit the world gracefully.
- Fixed in-game sounds playing inside menus after pressing ESC on a warning screen.

.github/workflows/build.yml

@@ -5,6 +5,7 @@
 
 name: build
 on:
+  - pull_request
   - workflow_call
   - workflow_dispatch
 

README.md

@@ -8,23 +8,16 @@ Like those commands, the menus will only be accessible in worlds with cheats/com
 Unlike the vanilla `/datapack` command, the Datapack menu from this mod can be used to toggle experimental features!
 
 
-## Word of caution: Datapacks
+## About Datapacks
 
-Some types of datapacks require a world restart to fully take effect.
-Whenever possible, the mod will show a warning after toggling one of them, and offer you the option to either back-out, or exit the world gracefully.
-However, some type of packs cannot be detected, and no warning will be displayed for those.
+**Experimental features** require a world restart in order to fully take effect. The mod will detect whenever you try to toggle one, and display a warning before exiting the world gracefully.
 
-### Registry Packs
-The new type of packs introduced in MC 1.21, those packs which add data to registries (painting variants, etc), **are not detected by the mod**, but still require a world restart to fully take effects.
+Since MC 1.21, some regular datapack can add data to registries (painting variants, etc), which also requires a world restart. However, **those packs are not detected by the mod**, so a restart cannot be enforced for those.
+A confirmation screen will still be displayed for any and all pack change, giving you the option to exit the world instead of hot-reloading.
 
-Toggling these packs may cause some errors in the log, but those are benign so long as you restart the world immediately afterward. This behaviour is no different from using the `/datapack` command.
-
-### Experimental Features
-Packs that include experimental features (such as bundles) are properly detected by the mod. Toggling them will also toggle the corresponding feature-flag on the world, and exit the world gracefully.
+Toggling a registry pack without restarting may print some errors in the log, but those are usually benign. This behaviour is no different from using the `/datapack` command.
 
 ### Vanilla Datapack
-The Vanilla datapack can technically be disabled, but you probably don't want to do it.
-Doing so will usually break worlds unless you know exactly what you are doing.
+The Vanilla datapack can technically be disabled, but doing so will break worlds unless you know exactly what you are doing.
 An additional warning screen will appear when trying to disable this pack.
-
 If you can't load a world after having disabled the Vanilla datapack, loading it in Safe Mode should be able to restore it.

build.gradle

@@ -37,6 +37,7 @@ loom {
 		clientClothless {
 			inherit client
 			configName "Client (Clothless)"
+			runDir "run/client"
 			property("fabric.debug.disableModIds", "cloth-gamerules")
 		}
 		server {

changelog.md

@@ -30,3 +30,7 @@
 - Added Traditional Chinese (zh_tw)
 ### 2.0.2
 - Updated for MC 1.21.2
+### 2.1.0
+- Added an optional confirmation screen for all datapack changes. This includes an option to exit the world gracefully.
+- Fixed in-game sounds playing inside menus after pressing ESC on a warning screen.
+

gradle.properties

@@ -12,7 +12,7 @@ loader_version=0.16.7
 fabric_version=0.106.1+1.21.3
 
 # Mod Properties
-mod_version=2.0.2
+mod_version=2.1.0
 maven_group=fr.estecka.packrulemenus
 archives_base_name=packrule-menus
 

src/main/java/fr/estecka/packrulemenus/DatapackHandler.java

@@ -1,7 +1,6 @@
 package fr.estecka.packrulemenus;
 
 import java.util.Collection;
-import it.unimi.dsi.fastutil.booleans.BooleanConsumer;
 import net.minecraft.client.MinecraftClient;
 import net.minecraft.client.gui.screen.MessageScreen;
 import net.minecraft.client.gui.screen.Screen;
@@ -25,27 +24,25 @@ public class DatapackHandler
 {
 	private final Screen parent;
 	private final IntegratedServer server;
-	private final MinecraftClient client = MinecraftClient.getInstance();
+	private final ResourcePackManager manager;
+	private final Collection<String> rollback;
+	static private final MinecraftClient client = MinecraftClient.getInstance();
 
-	public DatapackHandler(Screen parent, IntegratedServer server){
+	private DatapackHandler(Screen parent, IntegratedServer server){
 		this.parent = parent;
 		this.server = server;
+		this.manager = server.getDataPackManager();
+		this.rollback = manager.getEnabledIds();
 	}
 
-	private void	RevertScreen(){
-		client.setScreen(parent);
-	};
-
-	public ButtonWidget CreateButton(){
+	static public ButtonWidget CreateButton(Screen parent, IntegratedServer server){
 		return ButtonWidget.builder(
 				Text.translatable("selectWorld.dataPacks"),
-				__->client.setScreen( CreateScreen() )
+				__->client.setScreen( new DatapackHandler(parent, server).CreateScreen() )
 			).build();
 	}
 
 	public PackScreen CreateScreen(){
-		Collection<String> rollback = server.getDataPackManager().getEnabledIds();
-
 		return new PackScreen(
 			server.getDataPackManager(),
 			manager -> { HandleDatapackRefresh(manager, rollback); },
@@ -58,37 +55,23 @@ private void	HandleDatapackRefresh(final ResourcePackManager manager, Collection
 		FeatureSet neoFeatures = manager.getRequestedFeatures();
 		FeatureSet oldFeatures = server.getSaveProperties().getEnabledFeatures();
 
-		if (neoFeatures.equals(oldFeatures)) {
-			ReloadPacks(manager);
-			RevertScreen();
-		}
-		else {
+		if (!neoFeatures.equals(oldFeatures)){
 			boolean isExperimental = FeatureFlags.isNotVanilla(neoFeatures);
 			boolean wasVanillaRemoved = oldFeatures.contains(FeatureFlags.VANILLA) && !neoFeatures.contains(FeatureFlags.VANILLA);
-			BooleanConsumer onConfirm = confirmed -> {
-				if (confirmed){
-					this.ApplyFlags(manager);
-					this.server.stop(false);
-					if (this.client.world != null)
-						this.client.world.disconnect();
-					this.client.disconnect(new MessageScreen(Text.translatable("menu.savingLevel")));
-					this.client.setScreen(new TitleScreen());
-				} else {
-					manager.setEnabledProfiles(rollback);
-					RevertScreen();
-				}
-			};
-
-			client.setScreen(FeatureWarning(isExperimental, confirmed -> {
-				if (!wasVanillaRemoved || !confirmed)
-					onConfirm.accept(confirmed);
-				else
-					client.setScreen(VanillaWarning(onConfirm));
-			}));
+			ShowFeatureWarning(isExperimental, wasVanillaRemoved);
 		}
+		else if (PackRuleMod.CONFIG.datapackConfirmation)
+			ShowConfirmationScreen();
+		else
+			ReloadPacks();
 	}
 
-	private void	ApplyFlags(final ResourcePackManager manager){
+
+/******************************************************************************/
+/* ## Utility                                                                 */
+/******************************************************************************/
+
+	private void	ApplyFlags(){
 		FeatureSet features = manager.getRequestedFeatures();
 
 		String featureNames = "";
@@ -99,36 +82,74 @@ private void	ApplyFlags(final ResourcePackManager manager){
 		server.getSaveProperties().updateLevelInfo(new DataConfiguration(IMinecraftServerMixin.callCreateDataPackSettings(manager, true), features));		
 	}
 
+	private void SaveAndQuit(){
+		this.ApplyFlags();
+		this.server.stop(false);
+		if (client.world != null)
+			client.world.disconnect();
+		client.disconnect(new MessageScreen(Text.translatable("menu.savingLevel")));
+		client.setScreen(new TitleScreen());
+	}
 
-	private void	ReloadPacks(final ResourcePackManager manager){
+	private void	ReloadPacks(){
 		client.inGameHud.getChatHud().addMessage(Text.translatable("commands.reload.success"));
 
 		server.reloadResources(manager.getEnabledIds()).exceptionally(e -> {
 			PackRuleMod.LOGGER.error("{}", e);
 			client.inGameHud.getChatHud().addMessage(Text.translatable("commands.reload.failure").formatted(Formatting.RED));
 			return null;
 		});
+		client.setScreen(parent);
 	}
 
-	static public GenericWarningScreen	FeatureWarning(boolean isExperimental, BooleanConsumer onConfirm){
+	private void Rollback(){
+		this.manager.setEnabledProfiles(rollback);
+		client.setScreen(parent);
+	}
+
+
+/******************************************************************************/
+/* ## Warning Screens                                                         */
+/******************************************************************************/
+
+	public void	ShowConfirmationScreen(){
+		client.setScreen(new GenericWarningScreen(
+			Text.translatable("packrulemenus.warning.packConfirmation.title"),
+			Text.translatable("packrulemenus.warning.packConfirmation.message"),
+			Text.translatable("packrulemenus.warning.packConfirmation.checkbox"),
+			false,
+			checked -> { if(checked) SaveAndQuit(); else ReloadPacks(); },
+			this::Rollback
+		));
+	}
+
+	/**
+	 * @param isExperimental Whether the currently selected packs include
+	 * experimental features. This will be false upon removing all features.
+	 */
+	public void	ShowFeatureWarning(boolean isExperimental, boolean wasVanillaRemoved){
 		MutableText msg = Text.translatable("packrulemenus.warning.featureflag.message");
 		if (isExperimental)
 			msg.append("\n\n").append(Text.translatable("selectWorld.experimental.message"));
 
-		return new GenericWarningScreen(
+		client.setScreen(new GenericWarningScreen(
 			Text.translatable("packrulemenus.warning.featureflag.title"),
 			msg,
 			Text.translatable("packrulemenus.warning.featureflag.checkbox"),
-			onConfirm
-		);
+			true,
+			checked -> { if(wasVanillaRemoved) ShowVanillaWarning(); else if (checked) SaveAndQuit(); },
+			this::Rollback
+		));
 	}
 
-	static public GenericWarningScreen	VanillaWarning(BooleanConsumer onConfirm){
-		return new GenericWarningScreen(
+	public void	ShowVanillaWarning(){
+		client.setScreen(new GenericWarningScreen(
 			Text.translatable("packrulemenus.warning.vanillapack.title"),
 			Text.translatable("packrulemenus.warning.vanillapack.message"),
 			Text.translatable("packrulemenus.warning.vanillapack.checkbox"),
-			onConfirm
-		);
+			true,
+			checked -> { if(checked) SaveAndQuit(); },
+			this::Rollback
+		));
 	}
 }

src/main/java/fr/estecka/packrulemenus/GameruleHandler.java

@@ -24,6 +24,10 @@ public GameruleHandler(Screen parent, IntegratedServer server){
 		this.server = server;
 	}
 
+	static public ButtonWidget CreateButton(Screen parent, IntegratedServer server) {
+		return new GameruleHandler(parent, server).CreateButton();
+	}
+
 	public ButtonWidget CreateButton() {
 		final GameRules worldRules = server.getOverworld().getGameRules();
 		final FeatureSet features = server.getOverworld().getEnabledFeatures();

src/main/java/fr/estecka/packrulemenus/ModMenu.java

@@ -7,13 +7,15 @@
 import net.minecraft.client.gui.screen.Screen;
 import net.minecraft.client.gui.tooltip.Tooltip;
 import net.minecraft.client.gui.widget.ButtonWidget;
+import net.minecraft.client.gui.widget.CheckboxWidget;
 import net.minecraft.client.gui.widget.CyclingButtonWidget;
 import net.minecraft.client.gui.widget.DirectionalLayoutWidget;
 import net.minecraft.server.integrated.IntegratedServer;
 import net.minecraft.text.Text;
-import fr.estecka.packrulemenus.config.ConfigLoader;
+import fr.estecka.packrulemenus.config.Config;
 import fr.estecka.packrulemenus.config.EButtonLocation;
 import fr.estecka.packrulemenus.gui.GenericOptionScreen;
+import static fr.estecka.packrulemenus.PackRuleMod.CONFIG;
 
 
 public class ModMenu
@@ -33,8 +35,8 @@ static public Screen ModMenuScreen(Screen parent){
 		boolean showWorldOptions = PackRuleMod.CanModifyWorld();
 		ButtonWidget packs, rules;
 		if (showWorldOptions){
-			packs = new DatapackHandler(screen, server).CreateButton();
-			rules = new GameruleHandler(screen, server).CreateButton();
+			packs = DatapackHandler.CreateButton(screen, server);
+			rules = GameruleHandler.CreateButton(screen, server);
 		}
 		else {
 			packs = ButtonWidget.builder( Text.translatable("selectWorld.dataPacks"), __->{} ).build();
@@ -47,35 +49,40 @@ static public Screen ModMenuScreen(Screen parent){
 		row.add(rules);
 		row.add(packs);
 
-		screen.AddWidget(CreateConfigButton());
 		screen.AddWidget(row);
+		screen.AddWidget(CreateCyclingButtonOption());
+		screen.AddWidget(CreateConfirmationToggle());
 
 		return screen;
 	}
 
-	static private CyclingButtonWidget<EButtonLocation> CreateConfigButton(){
+	static private CyclingButtonWidget<EButtonLocation> CreateCyclingButtonOption(){
 		var button = CyclingButtonWidget.builder(EButtonLocation::TranslatableName)
 			.values(EButtonLocation.values())
-			.initially(PackRuleMod.BUTTON_LOCATION)
+			.initially(CONFIG.buttonLocation)
 			.tooltip(ModMenu::GetConfigTooltip)
-			.build(Text.translatable("packrulemenus.config.buttonlocation"), ModMenu::OnButtonChanged)
+			.build(Text.translatable("packrulemenus.config.buttonlocation"), (widget,value)->{CONFIG.buttonLocation=value;})
 			;
 
 		button.setWidth(8 + 2 * button.getWidth());
 		return button;
 	}
 
-	static private Tooltip GetConfigTooltip(EButtonLocation e){
-		return Tooltip.of(Text.translatable(e.TranslationKey() + ".tooltip"));
+	static CheckboxWidget CreateConfirmationToggle(){
+		final var client = MinecraftClient.getInstance();
+		var checkbox = CheckboxWidget.builder(Text.translatable("packrulemenus.config.askPackConfirmation"), client.textRenderer)
+			// .tooltip(Tooltip.of(Text.translatable("packrulemenus.config.askPackConfirmation.tooltip")))
+			.callback((widget,checked)->{CONFIG.datapackConfirmation=checked;})
+			.build()
+			;
+
+		if (CONFIG.datapackConfirmation)
+			checkbox.onPress();
+
+		return checkbox;
 	}
 
-	static private void OnButtonChanged(CyclingButtonWidget<?> button, EButtonLocation value){
-		PackRuleMod.BUTTON_LOCATION = value;
-		try {
-			PackRuleMod.CONFIG_IO.Write(new ConfigLoader());
-		}
-		catch (IOException e){
-			PackRuleMod.LOGGER.error(e.getMessage());
-		}
+	static private Tooltip GetConfigTooltip(EButtonLocation e){
+		return Tooltip.of(Text.translatable(e.TranslationKey() + ".tooltip"));
 	}
 }

src/main/java/fr/estecka/packrulemenus/PackRuleMod.java

@@ -6,8 +6,7 @@
 import net.minecraft.client.MinecraftClient;
 import net.minecraft.server.integrated.IntegratedServer;
 import fr.estecka.packrulemenus.config.ConfigIO;
-import fr.estecka.packrulemenus.config.ConfigLoader;
-import fr.estecka.packrulemenus.config.EButtonLocation;
+import fr.estecka.packrulemenus.config.Config;
 
 
 public class PackRuleMod
@@ -16,12 +15,12 @@ public class PackRuleMod
 	static public final Logger LOGGER = LoggerFactory.getLogger(MODID);
 
 	static public final ConfigIO CONFIG_IO = new ConfigIO(MODID+".properties");
-	static public EButtonLocation BUTTON_LOCATION = EButtonLocation.OPTIONS_HEADER;
+	static public final Config CONFIG = new Config();
 
 	static
 	{
 		try {
-			CONFIG_IO.GetIfExists(new ConfigLoader());
+			CONFIG_IO.GetIfExists(CONFIG);
 		}
 		catch (IOException e){
 			LOGGER.error(e.getMessage());

src/main/java/fr/estecka/packrulemenus/config/Config.java

@@ -0,0 +1,21 @@
+package fr.estecka.packrulemenus.config;
+
+import java.util.HashMap;
+import java.util.Map;
+import fr.estecka.packrulemenus.config.ConfigIO.Property;
+
+public class Config
+extends ConfigIO.AFixedCoded
+{
+	public EButtonLocation buttonLocation = EButtonLocation.OPTIONS_HEADER;
+	public boolean datapackConfirmation = true;
+
+	@Override
+	public Map<String, Property<?>> GetProperties(){
+		return new HashMap<>(){{
+			put("button.location", new Property<EButtonLocation>(()->buttonLocation, e->buttonLocation=e, EButtonLocation::parse, EButtonLocation::toString));
+			put("datapack.askConfirmation", Property.Boolean(()->datapackConfirmation, b->datapackConfirmation=b));
+		}};
+	}
+
+}