Repository navigation
Conversation
…bedrock pack conversion
…BedrockComponent interface
… enchantable and tool configurations
…ON serialization Signed-off-by: 1robie <97293924+1robie@users.noreply.github.com>
…rface and JSON serialization Signed-off-by: 1robie <97293924+1robie@users.noreply.github.com>
…ration with BedrockComponent interface and JSON serialization Signed-off-by: 1robie <97293924+1robie@users.noreply.github.com>
…ce and JSON serialization Signed-off-by: 1robie <97293924+1robie@users.noreply.github.com>
…rationSection handling
… improved retrieval logic
…prove type safety
…ect model configuration classes
…h texture data handling
…nt HasComponentPredicate
… and new range dispatch types
# Conflicts: # API/pom.xml # API/src/main/java/fr/robie/craftengineconverter/api/configuration/item/components/ConsumableConfiguration.java # API/src/main/java/fr/robie/craftengineconverter/api/configuration/item/components/SwingAnimationConfiguration.java # API/src/main/java/fr/robie/craftengineconverter/api/format/Message.java # API/src/main/java/fr/robie/craftengineconverter/api/manager/FileCacheManager.java # Common/src/main/java/fr/robie/craftengineconverter/common/utils/SnakeUtils.java # Plugin/src/main/java/fr/robie/craftengineconverter/CraftEngineConverter.java # Plugin/src/main/java/fr/robie/craftengineconverter/command/CraftEngineConverterCommand.java # Plugin/src/main/java/fr/robie/craftengineconverter/converter/Converter.java # Plugin/src/main/java/fr/robie/craftengineconverter/converter/itemsadder/IAConverter.java # Plugin/src/main/java/fr/robie/craftengineconverter/converter/nexo/NexoConverter.java # Plugin/src/main/java/fr/robie/craftengineconverter/converter/nexo/NexoItemConverter.java # Plugin/src/main/java/fr/robie/craftengineconverter/database/DataBaseManager.java # Plugin/src/main/java/fr/robie/craftengineconverter/loader/MessageLoader.java # Plugin/src/main/java/fr/robie/craftengineconverter/utils/JsonFileValidator.java
…nside MinecraftVersion class
ItemConversionContext kept its converters in maps keyed by the raw item id. ItemsAdder scopes item ids per namespace, so two packs may both define "ruby": the second put overwrote the first, and the losing item was never converted, never serialized and never registered in PluginNameMapper. It disappeared with no diagnostic at all. The maps are now keyed by the final id, which is what actually has to be unique. A genuine collision (same id twice in one namespace, or two same-named files in different Nexo subfolders) is reported and the first definition kept instead of being silently clobbered. Dependencies are still declared as raw ids, so idByRawId translates them back, claiming on a first-come basis and reporting the ambiguity. Only Nexo declares dependencies today, and Nexo ids are globally unique, so the mapping there stays one to one. For ItemsAdder, PluginNameMapper still gets both an unqualified and a "namespace:id" entry, but the unqualified one no longer silently changes meaning: the first namespace to claim it wins and later claims are reported. A qualified reference always resolves correctly. The raw-id-keyed accessors are dropped along with the other unused ones, since their contract no longer means anything; the post processors read the final id off the converter, which has always carried it.
injectResolvedDependency walked every converter and called getDependencies() on each one, and it runs once per converted item. NexoItemConverter#getDependencies re-reads its configuration section and allocates a fresh list on every call, so a pack of n items performed n^2 of those reads - a few million on a large pack, all to answer a question whose answer never changes. Dependencies are now collected once at scan time, along with the inverse index, so injection is proportional to the number of actual dependants. topologicalSort's cycle fallback also used List#contains inside a filter over every item; it uses a set now.
Nexo lets an item inherit from one or more others via `template`, and
skips any item that is named as a template rather than emitting it. The
converter read neither: templated items lost everything they inherited -
often their material, model and pack section - and the template stubs
were converted into real items alongside them.
NexoItemTemplates reproduces what Nexo's ItemParser does:
- templates are indexed globally, so an item can inherit from one
declared in another file, and `template` also answers to `templates`
- several templates merge in declaration order, the item merges on top,
and nested sections merge key by key rather than being replaced
- templates may name templates; a cycle is reported and broken instead
of recursing forever
- the placeholders a template is written against - <item_id>,
<item_id_capitalized>, <lore>, <parent>, <model>, <texture> - resolve
against the inheriting item, including splicing that item's own lore
into the position the template chose for it
- `injectId` is dropped from the inherited section
Resolution works on a detached copy, so nothing is written back into the
loaded configuration, and template ids are excluded from both the item
key set and the progress-bar total.
Both converters dispatched on a switch whose default arm was empty, so eleven of Nexo's thirteen mechanics and every ItemsAdder behaviour bar two were dropped without a word. The same was true of whole item-level keys - Nexo's PotionEffects and trim, ItemsAdder's potion and book properties - and of most of Nexo's furniture and custom-block sub-keys. Nothing in the output said so, so a pack could convert "successfully" and quietly lose half of what it configured. Converters now record what they skipped and print it once at the end, as one line per feature with a count and a few example items: a mechanic shared by two thousand items is one line, not two thousand warnings. The unread keys are derived by diffing the item's actual configuration against the set of keys each converter reads, rather than against a list of what the source plugin supports. A key nobody thought to handle is therefore reported by default, instead of needing to be remembered.
restricted_rotation is Nexo's placement snap: VERY_STRICT turns in 45
degree steps, STRICT in 22.5, and the key defaults to STRICT when absent
or unrecognised. The converter mapped VERY_STRICT and STRICT to FOUR and
EIGHT - 90 and 45 degrees - so every piece of furniture snapped to half
the directions it should, and NONE was not handled at all.
CraftEngine's RotationRule uses the same steps, so the mapping is now
direct: NONE to ANY, VERY_STRICT to EIGHT, STRICT to SIXTEEN.
`rotatable` no longer takes part. It is a {rotatable, on_sneak} pair
controlling whether a player may turn furniture that is already placed,
which has nothing to do with placement snapping and no equivalent in
CraftEngine's placement rules; it is reported as unconverted instead.
Thirty configuration classes emitted their vanilla data components onto a `components` section on the item itself. CraftEngine never looks there: AbstractItemManager feeds only the item's `data` section to ItemProcessors, and raw components belong under `data.components`. The output parsed as valid YAML and was then ignored in full, so every converted food, consumable, equippable, tool, max_damage, use_cooldown, weapon, jukebox_playable and custom_data component did nothing at all. Verified against the CraftEngine version this converter targets (26.5, commit ca314a81d) as well as current 26.8: neither reads a top-level `components`, and CraftEngine's own bundled configs put it under `data`. Both destinations now go through getDataSection/getComponentsSection on ItemConfigurationSerializable rather than being spelled out at each call site, and ItemDataPlacementTest pins the placement - it fails on the previous output.
Three item-level keys Nexo reads that the converter did not:
- `customname` is the display name written onto the stack, distinct
from `itemname`, which is the item_name component. Items relying on
it converted with no name at all.
- `trim_material` and `trim_pattern` become CraftEngine's `data.trim`,
which has its own processor and needs both halves; half a trim is
reported rather than emitted.
- `PotionEffects` becomes a `minecraft:potion_contents` component, as
CraftEngine has no dedicated processor for it. The type is read from
`type` or `effect` - Nexo accepts the first and hands the rest of the
map to Bukkit's PotionEffect, which spells it the second way - and
only flags that were actually set are written, so a plain effect
stays three keys instead of carrying vanilla's defaults explicitly.
Three of the specific_properties the converter ignored:
- `potion` becomes a minecraft:potion_contents component. The base
`type` goes through Bukkit's PotionType so it lands on the vanilla
registry name - SPEED is minecraft:swiftness, not minecraft:speed -
and the effects are read as a section keyed by arbitrary labels,
which is how ItemsAdder writes them, with its own flag defaults
(ambient false, particles and icon true).
- `book` becomes CraftEngine's written_book_content processor, titled
from the item's display name the way ItemsAdder titles it, with the
Generation name mapped to vanilla's number.
- `leather_horse_armor.color` now feeds the dyed colour, which
previously only looked at graphics.color.
The remaining specific_properties - bow, crossbow and seed - are
gameplay behaviours with no component or processor behind them; they
stay in the unconverted report.
The converter read seats[0], parsed it, and attached it to whichever shulker or happy-ghast hitbox happened to be built first. A bench with three seats therefore converted with one, and furniture whose hitboxes are all interactions converted with none at all. Every entry is parsed now, and they are attached once the hitboxes exist rather than while they are being built - so the hitbox type no longer decides whether seats survive. They all hang off a single hitbox because CraftEngine keys seats by their offset and shares them across a furniture's hitboxes, and Nexo's offsets are relative to the furniture origin rather than to any one hitbox. Seats configured on furniture with no hitbox at all are reported instead of silently dropped.
Nexo's furniture storage mechanic had no counterpart in the output, so a chest, barrel or cabinet converted into furniture you cannot open. CraftEngine registers simple_storage_furniture for exactly this, and it takes the same four things Nexo configures - rows, title, and the open and close sounds - with Nexo's defaults (6 rows, "Storage", the vanilla chest sounds) carried over. Furniture behaviours reach CraftEngine as a list of maps keyed by type rather than as a section each, so FurnitureConfiguration grew a behavior list to serialize them that way. Only Nexo's plain STORAGE type maps across. PERSONAL, ENDERCHEST, DISPOSAL and SHULKER open a player's own inventory, a bin or a shulker rather than storage the furniture owns, and are reported by type instead of being converted into something they are not.
A pass over the block, furniture and chunk-load converters. Five
separate defects, kept in one commit because they overlap in the same
handful of files:
- executeFurnitureConversion read getAsString() off an entity it had
already allowed to be null. isFurnitureAt and getFurnitureEntityAt
do not have to agree - Nexo answers the first from barrier blocks,
and the two are asked about locations half a block apart - so
propagating a conversion past furniture with no nearby ItemDisplay
threw. It now converts without the snapshot rather than dying.
- ItemsAdderFurnitureConverter#getNewNameForFurniture returned the
ItemsAdder id instead of what it converts to, so the propagation
check looked the raw id up in CraftEngine, never found it, and gave
up. ItemsAdder furniture propagation has therefore never converted
anything. Its block counterpart already mapped the id.
- The ItemsAdder chunk-load converter removed the entity and placed
replacement furniture without checking CraftEngine knows the id, so
an unconverted item silently destroyed the furniture. It also placed
at the entity's own position while every other path offsets half a
block down, leaving chunk-loaded furniture floating.
- PluginNameMapper looked ids straight up in a ConcurrentHashMap,
which throws on null. Both chunk-load converters feed it whatever a
persistent data container hands back, which is null if the value was
not a string.
- Neither ItemsAdder converter recorded any history, so
`worldconverter restore` had nothing to undo for ItemsAdder while
Nexo restored fine. Recording moved into BlockConverter and
FurnitureConverter, where both hooks get it, and the ItemsAdder
handlers now read the block state and entity NBT before destroying
what they describe. Nexo's near-identical overrides are gone.
Three things the previous pass left open, all in how a conversion spreads to what adjoins it. The furniture walk could not remember where it had been. It keyed the visited set on Location, which compares exact coordinates, while the same furniture is reached both through its entity's fractional position and through a whole-block offset from a neighbour - two values that never compare equal. So it revisited the same pieces until the budget ran out, cutting the reach short and wasting most of it. Both walks now key on the block a position falls in. max-block-conversion-propagation-depth is not a depth. It caps how many objects one interaction converts, which is what its config comment has always said, so the key is renamed to -limit; existing configs carry their value across on the next start through the usual legacy-path move. getFurnitureEntityAt returned whichever candidate the world listed first. Both hooks search a box a block wide in every direction, where a neighbouring piece is as likely to be found as the one being asked about, so the nearest one wins now. The four hooks also repeated the same six lines to set propagation up. That moved to the bases as propagateFrom, which is what made the rename a three-line change instead of an eight-site one.
saveAll ran once a minute and wrote a single batch, so the buffer emptied at 250 rows per minute. A world conversion produces rows far faster than that, so it grew for as long as the conversion ran - every pending row and its key held in memory, and roughly 66 hours of draining behind a million converted blocks. A save cycle now drains until the buffer is empty, batched at 250 rows per insert as before and capped per cycle so one pass cannot monopolise the connection. The timer became a floor rather than the only trigger: a cache that builds up past a threshold asks to be drained straight away, off the main thread. processBatch reports what it wrote, which is what lets a drain know it is making progress, and flush is now that loop rather than its own nested copy - retrying rather than giving up if a scheduled drain holds the lock, since dropping rows at shutdown is the one outcome worth waiting for.
ServerProfileManager loaded every non-reverted row into a Map<String, BlockHistory> at startup and kept it there for the life of the server. Measured at 375 bytes a block: a String key spelling out the coordinates, a per-row copy of the world name, and the block state string - none of which the index is ever asked about. It answers one question, "has this been converted?", and the record behind a position is only wanted when something is restored, which can read it back. Blocks are now a LongOpenHashSet per world of positions packed the way Minecraft packs a BlockPos - 26 bits of x, 12 of y, 26 of z, which fits any world Minecraft can generate with room to spare. Measured at 18.9 bytes a block across three worlds, so a five million block conversion holds 90 MB where it used to hold 1.8 GB. Entity conversions drop their records too, keeping the location strings exact: there is one per piece of furniture rather than one per block, so they are not worth packing, but their NBT snapshots were the largest single thing in the old cache. Building the index no longer materialises what it is reading either. Sarah's select collects every matching row before returning, which would have spiked to the old figure just to populate the new index, so the two startup reads stream over a JDBC cursor and project only the columns they need. clearBlockHistory became one UPDATE rather than one per row.
The restore read every active conversion into a list, copied it, then scheduled one task per fifty rows before doing any work - so a million converted blocks meant a million records resident at once and twenty thousand tasks queued up front, on top of an update per row as they went. It now walks the table a page at a time, each page scheduling the next: constant memory and one pending task no matter how large the world. The walk pages by id rather than by offset, which is what keeps it correct while it marks rows reverted underneath itself - an offset would skip a page every time. Reads and reverts happen off the main thread, with only the block and entity changes on it. A page per tick against a remote database would otherwise have stalled the server for a round trip every tick for the whole restore. Each page is reverted in a single statement rather than row by row. ServerProfile#getAllActiveConversions and its entity twin are gone. The index no longer holds records to hand back, and an API whose contract is "give me every row" is the thing this set out to remove; forgetBlock and forgetEntity replace the part of markAsReverted the paged path still needs, without a second update for a row the page already covered.
Chunks already scanned were remembered as a record per chunk, each carrying a world-name reference and an object header to hold two ints, in a set nothing ever evicts. A server up long enough to walk a million chunks paid tens of megabytes for a list of "already looked at this". They are packed x and z into one long per world now, the same shape the converted-block index uses. ChunkPosition had no other caller and goes with it.
A sweep over the rest of the plugin for the same shapes the database work turned up: state shared across threads without saying so, and things held in memory after they were finished with. WorldConverterManager kept its in-flight conversions in an ArrayList. Chunks are scanned on their region's thread and each conversion removes itself from a completion callback on whichever thread finished it, while the status and shutdown paths walk the list - three-way concurrent access on a collection that supports none. It is a concurrent set now, which also makes the per-completion removal O(1) rather than a scan. SimpleCache was a bare HashMap behind a static field. Its one user maps image names for chat formatting and placeholders: filled by a conversion on its own thread, read from whichever thread is rendering a message. It is a ConcurrentHashMap now, and tolerates the null keys that a ConcurrentHashMap would otherwise throw on, since the keys come from whatever text a player sent. ConversionContext kept every icon it rendered as a decoded raster for the length of a conversion - roughly sixteen kilobytes each at the default size and sixteen times that at the largest, for images that have already been written to disk. They cannot be dropped after their first reader, because several items can share one, so they are kept as encoded PNG instead: a tenth of the size, and the form they get written in anyway, so assigning one is now a byte copy rather than a re-encode.
The queries added for the memory work were written by hand rather than built by the ORM, so nothing checked them: a mistyped column, a paging clause that skips rows and a boolean a driver spells differently all compile the same. They only run at startup and during a restore, which is the worst place to find out. They move to ConversionHistoryQueries, which takes a connection instead of a plugin, and run against a real SQLite database - the storage the plugin ships with by default. The paging test does exactly what the restore does, reading a page and reverting it before asking for the next, and checks that all 250 rows are visited exactly once and none survives; another checks the walk still moves on past a page it could not revert, which is the case that would otherwise loop for ever. The block index came out of ServerProfileManager for the same reason, so one test can carry rows from the database through the real queries into the real index and assert what comes out - which is the only thing that shows those four columns arrive in the order the index reads them. IconEncodingTest pins the other assumption from that work: that encoding an icon once and writing the bytes later produces the same PNG as encoding it at the point of writing.
CraftEngine now ships its own Bedrock converter, so this plugin no longer carries one. Removes the converter, its tests and fixtures, the bedrock.yml settings file, the /cec bedrock command, the devConvert task, the Bedrock developer tool and roadmap. Settings that item converters still read (disable-default-italic, armor-converter-type, default-material) move from bedrock.yml into config.yml. The Bedrock-only settings are dropped.
Gradle 9.6.1 -> 9.9.0 (wrapper, with checksum), run-paper 3.0.2 -> 3.1.0, adventure 4.26.1 -> 5.2.0, folialib 0.5.1 -> 0.5.2, itemsadder 4.0.17 -> 4.0.18-beta-10, junit 6.0.3 -> 6.1.3, nexo 1.20.1 -> 1.29.1, packetevents 2.11.2 -> 2.14.0, paperdispatch 1.0.2 -> 1.1.0, placeholderapi 2.12.2 -> 2.12.3, sarah 1.23 -> 1.27, snakeyaml 2.6 -> 2.7. Held back on purpose: - craftengine stays on 26.5-SNAPSHOT: 26.10 moved or removed the AttributeModifier and Instrument types the API module uses. - paper stays on 1.21.11: paper-api 26.3 needs Java 25, and the build targets Java 21.
26.10 moved the attribute types to attribute.vanilla.VanillaAttributeModifier, removed core.util.Instrument, and changed AnvilRepairItem to take ItemSelector[] targets and NumberProvider amounts. - AttributeModifier and the converters use VanillaAttributeModifier. - BlockSettings uses org.bukkit.Instrument; nothing read the CraftEngine one. - AnvilRepairItemConfiguration maps ItemSelector back to the same string form, and reads amount/percent through NumberProvider. - Nexo passes constant NumberProviders for the fixed 1 / 1.0 values. Paper stays on 1.21.11 because paper-api 26.x needs Java 25.
# Conflicts: # pom.xml
develop is now the working branch; master only takes releases. Every push to develop posts a dev build as the rolling GitHub pre-release "dev". Dependabot moves from Maven to the Gradle catalog and targets develop. The version is now 0.0.3-SNAPSHOT so dev builds are numbered 0.0.3-dev.<run>.
CI runs ./gradlew on Linux, which needs the executable bit.
docs/ is local documentation only and is now ignored.
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Tip 🎯 Customize your risk assessments
✅ Automatic approvals, with your rules
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
Owner
Author
|
Check registered; closing (not a release). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Opened only so GitHub sees the
Release sourcecheck once, so the master ruleset can require it. Closed right after; not a release.