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

fix: 🐛 Fixed list config values to have proper validation #712

Merged
merged 1 commit into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ org.gradle.daemon=false

mod_id=reliquary
mod_group_id=reliquary
mod_version=2.0.44
mod_version=2.0.45
sonar_project_key=xreliquary:Reliquary
github_package_url=https://maven.pkg.github.com/P3pp3rF1y/Reliquary

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,9 +302,8 @@ private int getRedstoneAmpLimit() {

private Set<Block> getHeatSources() {
Set<Block> heatSources = new HashSet<>();
List<String> heatSourceBlockNames = Settings.COMMON.blocks.apothecaryCauldron.heatSources.get();

heatSourceBlockNames.forEach(blockName -> heatSources.add(ForgeRegistries.BLOCKS.getValue(new ResourceLocation(blockName))));
Settings.COMMON.blocks.apothecaryCauldron.heatSources.get()
.forEach(blockName -> heatSources.add(ForgeRegistries.BLOCKS.getValue(new ResourceLocation(blockName))));
//defaults that can't be removed.
heatSources.add(Blocks.LAVA);
heatSources.add(Blocks.FIRE);
Expand Down
7 changes: 2 additions & 5 deletions src/main/java/reliquary/entities/shot/ShotEntityBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,7 @@
import reliquary.util.potions.XRPotionHelper;

import javax.annotation.Nullable;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Optional;
import java.util.*;

@SuppressWarnings("squid:S2160")
public abstract class ShotEntityBase extends Projectile {
Expand Down Expand Up @@ -393,7 +390,7 @@ private void ricochet(Direction sideHit) {
*/
void seekTarget() {
Entity closestTarget = null;
List<String> huntableEntitiesBlacklist = Settings.COMMON.items.seekerShot.huntableEntitiesBlacklist.get();
Set<String> huntableEntitiesBlacklist = new HashSet<>(Settings.COMMON.items.seekerShot.huntableEntitiesBlacklist.get());
List<Entity> targetsList = level().getEntities(this,
new AABB(getX() - 5, getY() - 5, getZ() - 5, getX() + 5, getY() + 5, getZ() + 5),
Mob.class::isInstance);
Expand Down
15 changes: 2 additions & 13 deletions src/main/java/reliquary/items/MidasTouchstoneItem.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,7 @@
import com.google.common.collect.ImmutableMap;
import net.minecraft.world.entity.Entity;
import net.minecraft.world.entity.player.Player;
import net.minecraft.world.item.ArmorItem;
import net.minecraft.world.item.ArmorMaterial;
import net.minecraft.world.item.ArmorMaterials;
import net.minecraft.world.item.Item;
import net.minecraft.world.item.ItemStack;
import net.minecraft.world.item.Items;
import net.minecraft.world.item.Rarity;
import net.minecraft.world.item.Tier;
import net.minecraft.world.item.TieredItem;
import net.minecraft.world.item.Tiers;
import net.minecraft.world.item.*;
import net.minecraft.world.level.Level;
import net.minecraftforge.api.distmarker.Dist;
import net.minecraftforge.api.distmarker.OnlyIn;
Expand Down Expand Up @@ -81,8 +72,6 @@ public void inventoryTick(ItemStack stack, Level world, Entity e, int i, boolean
}

private void doRepairAndDamageTouchstone(ItemStack touchstone, Player player) {
List<String> goldItems = Settings.COMMON.items.midasTouchstone.goldItems.get();

InventoryHelper.getItemHandlerFrom(player, null).ifPresent(itemHandler -> {
for (int slot = 0; slot < itemHandler.getSlots(); slot++) {
ItemStack stack = itemHandler.getStackInSlot(slot);
Expand All @@ -92,7 +81,7 @@ private void doRepairAndDamageTouchstone(ItemStack touchstone, Player player) {
continue;
}

tryRepairingItem(touchstone, player, goldItems, stack, item);
tryRepairingItem(touchstone, player, Settings.COMMON.items.midasTouchstone.getGoldItems(), stack, item);
}
});
}
Expand Down
75 changes: 38 additions & 37 deletions src/main/java/reliquary/reference/Settings.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,32 +6,26 @@
import net.minecraft.world.entity.EntityType;
import net.minecraft.world.item.Items;
import net.minecraftforge.common.ForgeConfigSpec;
import net.minecraftforge.common.ForgeConfigSpec.BooleanValue;
import net.minecraftforge.common.ForgeConfigSpec.ConfigValue;
import net.minecraftforge.common.ForgeConfigSpec.DoubleValue;
import net.minecraftforge.common.ForgeConfigSpec.EnumValue;
import net.minecraftforge.common.ForgeConfigSpec.IntValue;
import net.minecraftforge.common.ForgeConfigSpec.*;
import net.minecraftforge.fml.event.config.ModConfigEvent;
import net.minecraftforge.registries.ForgeRegistries;
import org.apache.commons.lang3.tuple.Pair;
import reliquary.client.gui.hud.HUDPosition;

import javax.annotation.Nullable;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.*;
import java.util.function.Predicate;
import java.util.regex.Pattern;

import static reliquary.util.RegistryHelper.getItemRegistryName;

@SuppressWarnings("squid:S1192") //no issue repeating the same string literal as they are independent
@SuppressWarnings({"java:S4968", "squid:S1192"}) // ? extends String is the type parameter returned from defineList so it can't be just String here | no issue repeating the same string literal as they are independent
public class Settings {
private Settings() {}

private static final int ITEM_CAP = 9999;
private static final Pattern REGISTRY_NAME_PATTERN = Pattern.compile("([a-z0-9_.-]+:[a-z0-9_/.-]+)");
private static final Predicate<Object> REGISTRY_NAME_MATCHER = o -> o instanceof String s && REGISTRY_NAME_PATTERN.matcher(s).matches();

@SuppressWarnings("unused") // parameter needs to stay for addListener logic to recognize what this method is listening to
public static void onFileChange(ModConfigEvent.Reloading configEvent) {
Expand Down Expand Up @@ -354,7 +348,7 @@ public static class AngelHeartVialSettings {
public final DestructionCatalystSettings destructionCatalyst;

public static class DestructionCatalystSettings {
public final ConfigValue<List<String>> mundaneBlocks;
public final ConfigValue<List<? extends String>> mundaneBlocks;
public final IntValue gunpowderCost;
public final IntValue gunpowderWorth;
public final IntValue gunpowderLimit;
Expand All @@ -367,7 +361,7 @@ public static class DestructionCatalystSettings {

mundaneBlocks = builder
.comment("List of mundane blocks the catalyst will break")
.define("mundaneBlocks", Lists.newArrayList(
.defineList("mundaneBlocks", Lists.newArrayList(
"minecraft:dirt",
"minecraft:coarse_dirt",
"minecraft:podzol",
Expand All @@ -384,7 +378,7 @@ public static class DestructionCatalystSettings {
"minecraft:snow",
"minecraft:soul_sand",
"minecraft:netherrack",
"minecraft:end_stone"));
"minecraft:end_stone"), REGISTRY_NAME_MATCHER);

gunpowderCost = builder
.comment("Number of gunpowder it costs per catalyst use")
Expand Down Expand Up @@ -714,7 +708,6 @@ public static class InfernalClawsSettings {
public static class InfernalTearSettings {
private static final String ITEM_EXPERIENCE_MATCHER = "([a-z1-9_.-]+:[a-z1-9_/.-]+)\\|\\d+";
public final BooleanValue absorbWhenCreated;
@SuppressWarnings("java:S4968") // ? extends String is the type parameter returned from defineList so it can't be just String here
public final ForgeConfigSpec.ConfigValue<List<? extends String>> itemExperienceList;
@Nullable
private Map<String, Integer> itemExperience = null;
Expand Down Expand Up @@ -794,7 +787,7 @@ public static class KrakenShellSettings {
public final LanternOfParanoiaSettings lanternOfParanoia;

public static class LanternOfParanoiaSettings {
public final ConfigValue<List<String>> torches;
public final ConfigValue<List<? extends String>> torches;
public final IntValue minLightLevel;
public final IntValue placementScanRadius;

Expand All @@ -803,7 +796,7 @@ public static class LanternOfParanoiaSettings {

torches = builder
.comment("List of torches that are supported by the lantern")
.define("torches", Lists.newArrayList(getItemRegistryName(Items.TORCH)));
.defineList("torches", Lists.newArrayList(getItemRegistryName(Items.TORCH)), REGISTRY_NAME_MATCHER);
minLightLevel = builder
.comment("Minimum light level below which the lantern will place torches")
.defineInRange("minLightLevel", 1, 0, 15);
Expand All @@ -819,7 +812,7 @@ public static class LanternOfParanoiaSettings {
public final MidasTouchstoneSettings midasTouchstone;

public static class MidasTouchstoneSettings {
public final ConfigValue<List<String>> goldItems;
public final ConfigValue<List<? extends String>> goldItems;
public final IntValue glowstoneCost;
public final IntValue glowstoneWorth;
public final IntValue glowstoneLimit;
Expand All @@ -829,7 +822,7 @@ public static class MidasTouchstoneSettings {

goldItems = builder
.comment("Gold items that can be repaired by the touchstone")
.define("goldItems", new ArrayList<>());
.defineListAllowEmpty("goldItems", new ArrayList<>(), REGISTRY_NAME_MATCHER);

glowstoneCost = builder
.comment("Number of glowstone that the repair costs")
Expand All @@ -845,6 +838,10 @@ public static class MidasTouchstoneSettings {

builder.pop();
}

public List<String> getGoldItems() {
return getStringList(goldItems.get());
}
}

public final MobCharmSettings mobCharm;
Expand Down Expand Up @@ -1015,8 +1012,8 @@ public static class RendingGaleSettings {
public final BooleanValue canPushProjectiles;
public final IntValue pedestalFlightRange;
public final IntValue pedestalCostPerSecond;
public final ConfigValue<List<String>> pushableEntitiesBlacklist;
public final ConfigValue<List<String>> pushableProjectilesBlacklist;
public final ConfigValue<List<? extends String>> pushableEntitiesBlacklist;
public final ConfigValue<List<? extends String>> pushableProjectilesBlacklist;

RendingGaleSettings(ForgeConfigSpec.Builder builder) {
builder.comment("Rending Gale settings").push("rendingGale");
Expand Down Expand Up @@ -1059,11 +1056,11 @@ public static class RendingGaleSettings {

pushableEntitiesBlacklist = builder
.comment("List of entities that are banned from being pushed by the Rending Gale")
.define("pushableEntitiesBlacklist", new ArrayList<>());
.defineListAllowEmpty("pushableEntitiesBlacklist", new ArrayList<>(), REGISTRY_NAME_MATCHER);

pushableProjectilesBlacklist = builder
.comment("List of projectiles that are banned from being pushed by the Rending Gale")
.define("pushableProjectilesBlacklist", new ArrayList<>());
.defineListAllowEmpty("pushableProjectilesBlacklist", new ArrayList<>(), REGISTRY_NAME_MATCHER);

builder.pop();
}
Expand All @@ -1079,7 +1076,6 @@ public static class RodOfLyssaSettings {
public final BooleanValue failStealFromVacantSlots;
public final BooleanValue angerOnStealFailure;
public final BooleanValue stealFromPlayers;
private static final String ENTITY_NAME_MATCHER = "[a-z1-9_.-]+:[a-z1-9_/.-]+";
@SuppressWarnings("java:S4968") // ? extends String is the type parameter returned from defineList so it can't be just String here
public final ForgeConfigSpec.ConfigValue<List<? extends String>> entityBlockList;
@Nullable
Expand Down Expand Up @@ -1117,7 +1113,7 @@ public static class RodOfLyssaSettings {
.define("stealFromPlayers", true);

entityBlockList = builder.comment("List of entities on which lyssa rod doesn't work - full registry name is required here")
.defineList("entityBlockList", new ArrayList<>(), mapping -> ((String) mapping).matches(ENTITY_NAME_MATCHER));
.defineListAllowEmpty("entityBlockList", new ArrayList<>(), REGISTRY_NAME_MATCHER);
builder.pop();
}

Expand All @@ -1142,14 +1138,14 @@ private void initBlockedEntityTypes() {
public final SeekerShotSettings seekerShot;

public static class SeekerShotSettings {
public final ConfigValue<List<String>> huntableEntitiesBlacklist;
public final ConfigValue<List<? extends String>> huntableEntitiesBlacklist;

SeekerShotSettings(ForgeConfigSpec.Builder builder) {
builder.comment("Seeker Shot settings").push("seekerShot");

huntableEntitiesBlacklist = builder
.comment("Entities that are banned from being tracked by seeker shot")
.define("huntableEntitiesBlacklist", new ArrayList<>());
.defineListAllowEmpty("huntableEntitiesBlacklist", new ArrayList<>(), REGISTRY_NAME_MATCHER);

builder.pop();
}
Expand All @@ -1158,7 +1154,7 @@ public static class SeekerShotSettings {
public final SojournerStaffSettings sojournerStaff;

public static class SojournerStaffSettings {
public final ConfigValue<List<String>> torches;
public final ConfigValue<List<? extends String>> torches;
public final IntValue maxCapacityPerItemType;
public final IntValue maxRange;
public final IntValue tilePerCostMultiplier;
Expand All @@ -1168,7 +1164,7 @@ public static class SojournerStaffSettings {

torches = builder
.comment("List of torches that are supported by the staff")
.define("torches", getDefaultTorches());
.defineList("torches", getDefaultTorches(), REGISTRY_NAME_MATCHER);

maxCapacityPerItemType = builder
.comment("Number of items the staff can store per item type")
Expand Down Expand Up @@ -1286,7 +1282,7 @@ public static class AltarSettings {
public static class ApothecaryCauldronSettings {
public final IntValue redstoneLimit;
public final IntValue cookTime;
public final ConfigValue<List<String>> heatSources;
public final ConfigValue<List<? extends String>> heatSources;
public final IntValue glowstoneLimit;

ApothecaryCauldronSettings(ForgeConfigSpec.Builder builder) {
Expand All @@ -1302,7 +1298,7 @@ public static class ApothecaryCauldronSettings {

heatSources = builder
.comment("List of acceptable heat sources")
.define("heatSources", new ArrayList<>());
.defineListAllowEmpty("heatSources", new ArrayList<>(), REGISTRY_NAME_MATCHER);

glowstoneLimit = builder
.comment("Limit of glowstone that can be used in cauldron to make POTION more potent")
Expand Down Expand Up @@ -1343,8 +1339,8 @@ public static class FertileLilypadSettings {
public static class InterdictionTorchSettings {
public final IntValue pushRadius;
public final BooleanValue canPushProjectiles;
public final ConfigValue<List<String>> pushableEntitiesBlacklist;
public final ConfigValue<List<String>> pushableProjectilesBlacklist;
public final ConfigValue<List<? extends String>> pushableEntitiesBlacklist;
public final ConfigValue<List<? extends String>> pushableProjectilesBlacklist;

InterdictionTorchSettings(ForgeConfigSpec.Builder builder) {
builder.comment("Interdiction Torch settings").push("interdictionTorch");
Expand All @@ -1359,11 +1355,11 @@ public static class InterdictionTorchSettings {

pushableEntitiesBlacklist = builder
.comment("List of entities that are banned from being pushed by the torch")
.define("pushableEntitiesBlacklist", new ArrayList<>());
.defineListAllowEmpty("pushableEntitiesBlacklist", new ArrayList<>(), REGISTRY_NAME_MATCHER);

pushableProjectilesBlacklist = builder
.comment("List of projectiles that are banned from being pushed by the torch")
.define("pushableProjectilesBlacklist", new ArrayList<>());
.defineListAllowEmpty("pushableProjectilesBlacklist", new ArrayList<>(), REGISTRY_NAME_MATCHER);

builder.pop();
}
Expand Down Expand Up @@ -1435,4 +1431,9 @@ public static class PedestalSettings {
COMMON_SPEC = specPair.getRight();
COMMON = specPair.getLeft();
}

@SuppressWarnings("unchecked")
private static List<String> getStringList(List<? extends String> list) {
return (List<String>) list;
}
}
Binary file not shown.