P3-03 + P3-09 review fixes : tighten RL check + correct thread-safety doc
HIGH P3-03 : strict RL check laissait passer ':path', 'mod:', ':' silencieusement (tryParse les accepte en 1.20.1). Fix combo check (isEmpty || colonIdx<=0 || colonIdx>=length-1) rejette tous les cas edge. 3 nouveaux tests couvrent les malformed variants. MEDIUM P3-09 : javadoc de BondageStateHelpers affirmait ConcurrentHashMap alors que le backing V2EquipmentHelper utilise EnumMap via pattern MC main-thread + packet sync. Correction doc pour refléter la réalité.
This commit is contained in:
@@ -849,13 +849,19 @@ public final class DataDrivenItemParser {
|
|||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
String s = elem.getAsString();
|
String s = elem.getAsString();
|
||||||
// Strict RL parsing : animations MUST carry an explicit namespace
|
// Strict RL parsing : animations MUST carry an explicit namespace AND
|
||||||
// (e.g. "mymod:foo"). ResourceLocation.tryParse defaults bare paths
|
// path (e.g. "mymod:foo"). ResourceLocation.tryParse would silently
|
||||||
// to the "minecraft" namespace, which silently masks modder typos —
|
// accept ":path" as "minecraft:path", "mod:" with an empty path, or
|
||||||
// we reject strings without ':' here.
|
// ":" altogether — masking modder typos. Reject all of these here :
|
||||||
if (s.isEmpty() || s.indexOf(':') < 0) {
|
// - isEmpty() rejects ""
|
||||||
|
// - colonIdx <= 0 rejects "noNamespace" (-1) and ":path" (0)
|
||||||
|
// - colonIdx >= length - 1 rejects "mod:" (colon at end)
|
||||||
|
// The bare ":" is rejected by colonIdx == 0.
|
||||||
|
int colonIdx = s.indexOf(':');
|
||||||
|
if (s.isEmpty() || colonIdx <= 0 || colonIdx >= s.length() - 1) {
|
||||||
LOGGER.warn(
|
LOGGER.warn(
|
||||||
"[DataDrivenItemParser] Malformed ResourceLocation '{}' for {} in item {}, skipping.",
|
"[DataDrivenItemParser] Malformed ResourceLocation '{}' for {} in item {} "
|
||||||
|
+ "(expected 'namespace:path'), skipping.",
|
||||||
s,
|
s,
|
||||||
context,
|
context,
|
||||||
itemId
|
itemId
|
||||||
@@ -865,7 +871,8 @@ public final class DataDrivenItemParser {
|
|||||||
ResourceLocation rl = ResourceLocation.tryParse(s);
|
ResourceLocation rl = ResourceLocation.tryParse(s);
|
||||||
if (rl == null) {
|
if (rl == null) {
|
||||||
LOGGER.warn(
|
LOGGER.warn(
|
||||||
"[DataDrivenItemParser] Malformed ResourceLocation '{}' for {} in item {}, skipping.",
|
"[DataDrivenItemParser] Malformed ResourceLocation '{}' for {} in item {} "
|
||||||
|
+ "(invalid characters), skipping.",
|
||||||
s,
|
s,
|
||||||
context,
|
context,
|
||||||
itemId
|
itemId
|
||||||
|
|||||||
@@ -39,10 +39,11 @@ import net.minecraft.world.item.ItemStack;
|
|||||||
* {@code PacketSyncStruggleState}).</li>
|
* {@code PacketSyncStruggleState}).</li>
|
||||||
* </ul>
|
* </ul>
|
||||||
*
|
*
|
||||||
* <p><strong>Thread-safety</strong> : toutes les lectures backing sont
|
* <p><strong>Thread-safety</strong> : les APIs backing (V2EquipmentHelper,
|
||||||
* déjà thread-safe (volatile / ConcurrentHashMap / EntityData). Ces
|
* PlayerBindState) suivent le pattern MC standard — écriture serveur main
|
||||||
* helpers peuvent être appelés depuis les threads MC usuels (client
|
* thread, lecture client main thread après packet sync via
|
||||||
* tick, render thread) sans synchronisation additionnelle.</p>
|
* {@code Context.enqueueWork}. Pas de lock externe requis tant que
|
||||||
|
* l'appelant est sur le main thread approprié.</p>
|
||||||
*
|
*
|
||||||
* <p><strong>Note sur {@code @OnlyIn(Dist.CLIENT)}</strong> : ces helpers
|
* <p><strong>Note sur {@code @OnlyIn(Dist.CLIENT)}</strong> : ces helpers
|
||||||
* fonctionnent aussi server-side par construction (ils lisent les mêmes
|
* fonctionnent aussi server-side par construction (ils lisent les mêmes
|
||||||
|
|||||||
@@ -32,7 +32,7 @@ import com.tiedup.remake.rig.anim.TiedUpLivingMotions;
|
|||||||
* <li>bloc vide → {@link AnimationBindings#EMPTY},</li>
|
* <li>bloc vide → {@link AnimationBindings#EMPTY},</li>
|
||||||
* <li>motions vanilla EF et custom TiedUp!,</li>
|
* <li>motions vanilla EF et custom TiedUp!,</li>
|
||||||
* <li>tolerance typos (unknown motion) avec fuzzy-match suggestion,</li>
|
* <li>tolerance typos (unknown motion) avec fuzzy-match suggestion,</li>
|
||||||
* <li>tolerance ResourceLocation malformee,</li>
|
* <li>tolerance ResourceLocation malformee (no-namespace, leading/trailing/bare colon),</li>
|
||||||
* <li>tolerance values non-string,</li>
|
* <li>tolerance values non-string,</li>
|
||||||
* <li>one-shots on_equip / on_unequip,</li>
|
* <li>one-shots on_equip / on_unequip,</li>
|
||||||
* <li>Levenshtein distance (sanity check).</li>
|
* <li>Levenshtein distance (sanity check).</li>
|
||||||
@@ -218,6 +218,79 @@ class DataDrivenItemParserAnimationsTest {
|
|||||||
assertNull(result.livingMotions().get(LivingMotions.WALK));
|
assertNull(result.livingMotions().get(LivingMotions.WALK));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void parseAnimations_leadingColonRL_skipsEntry() {
|
||||||
|
// ":path" ne doit pas etre accepte silencieusement comme minecraft:path
|
||||||
|
// (ResourceLocation.tryParse le resoudrait sans le strict check).
|
||||||
|
String jsonStr = """
|
||||||
|
{
|
||||||
|
"animations": {
|
||||||
|
"living_motions": {
|
||||||
|
"WALK": ":arms_cuffed_walk",
|
||||||
|
"IDLE": "tiedup:arms_cuffed_idle"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
""";
|
||||||
|
|
||||||
|
AnimationBindings result =
|
||||||
|
DataDrivenItemParser.parseAnimationBindings(json(jsonStr), ITEM_ID);
|
||||||
|
|
||||||
|
assertNotNull(result);
|
||||||
|
assertEquals(1, result.livingMotions().size(),
|
||||||
|
"':path' rejete par strict check (colonIdx == 0)");
|
||||||
|
assertNotNull(result.livingMotions().get(LivingMotions.IDLE));
|
||||||
|
assertNull(result.livingMotions().get(LivingMotions.WALK));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void parseAnimations_trailingColonRL_skipsEntry() {
|
||||||
|
// "mod:" (empty path) doit etre rejete, tryParse l'accepterait.
|
||||||
|
String jsonStr = """
|
||||||
|
{
|
||||||
|
"animations": {
|
||||||
|
"living_motions": {
|
||||||
|
"WALK": "tiedup:",
|
||||||
|
"IDLE": "tiedup:arms_cuffed_idle"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
""";
|
||||||
|
|
||||||
|
AnimationBindings result =
|
||||||
|
DataDrivenItemParser.parseAnimationBindings(json(jsonStr), ITEM_ID);
|
||||||
|
|
||||||
|
assertNotNull(result);
|
||||||
|
assertEquals(1, result.livingMotions().size(),
|
||||||
|
"'mod:' rejete par strict check (colonIdx == length - 1)");
|
||||||
|
assertNotNull(result.livingMotions().get(LivingMotions.IDLE));
|
||||||
|
assertNull(result.livingMotions().get(LivingMotions.WALK));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void parseAnimations_bareColonRL_skipsEntry() {
|
||||||
|
// ":" seul doit etre rejete (colonIdx == 0 ET length - 1).
|
||||||
|
String jsonStr = """
|
||||||
|
{
|
||||||
|
"animations": {
|
||||||
|
"living_motions": {
|
||||||
|
"WALK": ":",
|
||||||
|
"IDLE": "tiedup:arms_cuffed_idle"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
""";
|
||||||
|
|
||||||
|
AnimationBindings result =
|
||||||
|
DataDrivenItemParser.parseAnimationBindings(json(jsonStr), ITEM_ID);
|
||||||
|
|
||||||
|
assertNotNull(result);
|
||||||
|
assertEquals(1, result.livingMotions().size(),
|
||||||
|
"':' rejete par strict check");
|
||||||
|
assertNotNull(result.livingMotions().get(LivingMotions.IDLE));
|
||||||
|
assertNull(result.livingMotions().get(LivingMotions.WALK));
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void parseAnimations_nonStringValue_skipsEntry() {
|
void parseAnimations_nonStringValue_skipsEntry() {
|
||||||
String jsonStr = """
|
String jsonStr = """
|
||||||
|
|||||||
Reference in New Issue
Block a user