Add Meteor 26.x parser profile support#1
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- StubGenerator: feed hand-written shim source dirs as exclusions, render nested-class references as Outer.Inner when outer is a manual shim. - Move legacy EntityTypeListSetting to profile-legacy and add 26x version using net.minecraft.world.entity.EntityType. - Add hand shim for WindowTabScreen and class_3417 in profile-26x, expand McpSchema with InitializeResult and ListToolsResult.
- Move all class_*-using Setting subclasses, Settings, SettingGroup, System, Names, SettingColor, and the class_2382 shim from src/main/java to src/profile-legacy/java. - Add Mojmap twins under src/profile-26x/java using CompoundTag, Block, Item, BlockPos, Holder, ResourceKey, SoundEvent, EntityType, Vec3i, Enchantment. - Setting (26x) now declares save/load against CompoundTag. - Add WindowScreen and Enchantment shims under profile-26x. - Remove meteor-villager-roller (1.21.11/Yarn) from the fixture set. src/main/java is now Yarn-free; 26x compiles clean against pure Mojmap fixtures.
Drop addons that still ship 1.21.x/Yarn jars (Baritone-Controller, mc-games, meteor-satellite-addon, Meteorist, MeteorPlus, nerv-printer-addon, Trouser-Streak). Add 26.x-confirmed entries (MeteorAdditions, PowHax, Seija-Printer) explicitly. CI now downloads only Mojmap fixtures.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a multi-profile architecture to support different Minecraft versions and mapping sets, specifically adding a new 26x (Mojmap) profile alongside the existing legacy (Yarn) profile. The changes include build script updates to handle profile-specific source sets, a large collection of new stub classes for the 26x profile, and refactoring of the StubGenerator and ValueNormalizer to be profile-aware. Feedback focuses on critical issues in the stub implementations, including registry key collisions in SimpleRegistry, class name collisions for anonymous placeholders in Modules, and several potential NullPointerException risks in setting parsers and builders. Additionally, a non-functional builder method in the legacy profile was identified.
| private final Map<ResourceLocation, T> values = new LinkedHashMap<>(); | ||
|
|
||
| public void add(T value) { | ||
| if (value != null) values.put(ResourceLocation.parse(value.getClass().getSimpleName().toLowerCase()), value); |
There was a problem hiding this comment.
Registry key generation using value.getClass().getSimpleName() is flawed for stubs. For classes like Item or EntityType that use static instances of the same type, all entries will share the same key (e.g., "item" or "entitytype"), causing collisions and overwriting previous entries in the registry. This effectively breaks registry-based lookups for most Minecraft objects in the 26x profile.
| if (value != null) values.put(ResourceLocation.parse(value.getClass().getSimpleName().toLowerCase()), value); | |
| if (value != null) values.put(ResourceLocation.parse("stub_" + Integer.toHexString(System.identityHashCode(value))), value); |
| Module placeholder = new Module(placeholderCategory, name, "") {}; | ||
| moduleInstances.put(placeholder.getClass(), placeholder); | ||
| getGroup(placeholderCategory).add(placeholder); | ||
| return placeholder; |
There was a problem hiding this comment.
Using an anonymous class for placeholders causes a collision in moduleInstances because all instances share the same class type (e.g., Modules$1). Subsequent calls to get(name) for different names will overwrite the previous placeholder in the map, meaning only the last created placeholder will be present in the registry. This breaks lookups and iteration for multiple unique placeholders.
|
|
||
| @Override | ||
| protected Double parseImpl(String str) { | ||
| try { |
There was a problem hiding this comment.
Double.parseDouble(str) will throw a NullPointerException if str is null. A null check should be added to ensure safe parsing during setting initialization or updates.
return str == null ? null : Double.parseDouble(str);|
|
||
| @Override | ||
| protected Integer parseImpl(String str) { | ||
| try { |
There was a problem hiding this comment.
Integer.parseInt(str) will throw a NullPointerException if str is null. A null check should be added to ensure safe parsing.
return str == null ? null : Integer.parseInt(str);| } | ||
|
|
||
| @SuppressWarnings("unchecked") | ||
| public Builder defaultValue(EntityType<?>... values) { |
There was a problem hiding this comment.
The defaultValue builder method will throw a NullPointerException if the values varargs array is null. A null check should be added before iteration.
if (values != null) for (EntityType<?> value : values) set.add(value);| public Builder defaultValue(net.minecraft.world.level.block.Block... blocks) { | ||
| return this; | ||
| } |
There was a problem hiding this comment.
This builder method is empty and does not set the default value, which will result in an empty list regardless of the arguments passed. It should initialize this.defaultValue with the provided blocks.
| public Builder defaultValue(net.minecraft.world.level.block.Block... blocks) { | |
| return this; | |
| } | |
| public Builder defaultValue(net.minecraft.world.level.block.Block... blocks) { | |
| this.defaultValue = new java.util.ArrayList<>(java.util.Arrays.asList(blocks)); | |
| return this; | |
| } |
- SimpleRegistry.add: key by identity hash instead of class simple name so multiple instances of the same class (e.g. Blocks.AIR vs Blocks.DIRT) don't overwrite each other. - Modules.get(String): cache placeholders in a name-keyed map instead of putting them in moduleInstances by anonymous class type, which collapses every placeholder onto Modules$1 and overwrites prior entries.
1 participant
Summary
Adds dual mapping profile plumbing and brings the default parser profile up to Meteor 26.x / Mojmap compatibility. Legacy Yarn/class_* support is retained behind the legacy profile source set.
Validation
Notes
Legacy runtime fixture tests require legacy fixture jars. The current fixture set is 26x/Mojmap.
Serialization cleanup for ISerializable/settings overrides is intentionally deferred to follow-up commits.