Fix four fidelity losses when converting an existing Mudlet package - #57
Open
TheLastDarkthorne wants to merge 4 commits into
Open
Fix four fidelity losses when converting an existing Mudlet package#57TheLastDarkthorne wants to merge 4 commits into
TheLastDarkthorne wants to merge 4 commits into
Conversation
A folder can be described two ways: as a directory under src/<type>/, and as an entry in the parent json. Both are needed together whenever the folder carries attributes of its own, since a directory name alone cannot express isActive, a script body or event handlers. createItems() synthesises a placeholder item for each directory in a path, carrying nothing but the name and default attributes. mergeDown() then merges the placeholder and the real declaration by name, but it keeps whichever it happened to visit first and only concatenates children, so the declaration's attributes were discarded whenever the placeholder came first. The effect is silent: a trigger group declared isActive="no" builds as active. Mark synthesised placeholders and let a real declaration win the merge, while keeping child order unchanged.
conditonLineDelta is Mudlet's line window for a chained trigger: how many lines after its parent fires the child may still match. That applies whether or not the trigger itself is multiline, and Mudlet's own UI exposes the spinner independently of the multiline checkbox. muddler only copied multilineDelta when multiline was "yes" and forced 0 otherwise, so importing a package silently flattened the window on every non-multiline trigger.
A colour pattern is expected as "<fg>,<bg>". Splitting on the comma and indexing [1] unconditionally meant any other form aborted the whole build with an ArrayIndexOutOfBoundsException and a stack trace that named neither the trigger nor the pattern, leaving nothing to search for in a large project. Report which trigger and what it received instead.
Keys are described by a "keys" string such as "ctrl+F1", which is convenient when authoring by hand but cannot express a package imported from existing Mudlet XML: that carries raw Qt key codes, and there was no way to pass them through, so imported keys lost their binding. Use options.keyCode and options.keyModifier when given, falling back to parsing the keys string as before.
Author
|
Forgot to mention, I'm converting the |
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.
Four independent fidelity fixes, found while converting an existing Mudlet package into a
muddler project. Each is small, self-contained and revertable on its own.
Fixes #52
Fixes #53
Fixes #54
Fixes #55
The commits
Keep declared folder attributes when merging with directory placeholders (#52)
createItems()synthesises a placeholder item for each directory in a path, carrying onlythe name and default attributes.
mergeDown()then merges the placeholder with the realdeclaration by name, but keeps whichever it visited first and only concatenates
children,so the declaration's attributes were dropped when the placeholder came first. Synthesised
placeholders are now marked, and a real declaration wins the merge; child order is
unchanged.
Honour multilineDelta for triggers that are not multiline (#53)
conditonLineDeltais Mudlet's line window for a chained trigger, which applies whether ornot the trigger is multiline — its UI exposes the spinner independently of the multiline
checkbox. It was only copied when
multilinewas"yes".Report a usable error for a malformed colour pattern (#55)
Splitting on
,and indexing[1]unconditionally turned any other form into anArrayIndexOutOfBoundsExceptionnaming neither the trigger nor the pattern. It now sayswhich trigger and what it received.
Accept explicit keyCode and keyModifier on keys (#54)
A
keysstring like"ctrl+F1"is convenient when authoring by hand, but a packageimported from Mudlet XML carries raw Qt codes and had no way to express them, so imported
keys lost their binding.
keyCode/keyModifierare used when given, falling back toparsing
keysas before.Not included
#56 (same-named siblings being dropped) has no fix here.
mergeDown/fullMergeare builtaround matching by name, and separating "directory placeholder that should merge" from "two
real items that should not" looks like a design decision rather than a small fix — happy to
attempt it given a steer on the preferred approach.
Verification
Runnable reproductions for all five issues, each with a passing control:
https://github.com/TheLastDarkthorne/mudlet-tooling-repros
Every case was run against the stock 1.1.0 release jar and against this branch; the four
fixed cases change behaviour and the control cases do not.
Beyond the minimal cases, this branch was exercised on the conversion that turned these up:
svof — 25 Mudlet modules, ~2,500
triggers, 120 aliases, 70 scripts. Each module was converted to a muddler project, built,
and the generated XML compared against the original
object-by-object (structure, names, scripts, patterns and pattern types, event handlers,
and the behavioural flags). All 25 modules round-trip with no differences using this branch.
On stock 1.1.0 the same comparison loses the
isActivestate of 8 trigger groups, zeroes 30multilineDeltavalues, drops the package's only key binding, and cannot build at allwithout working around the colour-pattern crash.
Note on the build
This branch is built and tested with the wrapper's own Gradle 7.5, so it carries no build
changes. (Gradle 7.5 cannot parse class file major version 63, so building on JDK 19+ needs
a newer wrapper — kept out of this PR as an unrelated concern, and verified separately to
produce byte-identical output.)