Repository navigation
feat!: migrate DEL/OSTW to the canonical workshop-rs Program API - #123
Merged
Merged
Conversation
…workshop-rs Program API Refs #107. workshop-rs =0.3.7 -> 1.4: the private arena WIR is gone, so all Workshop-facing code now builds and walks the public tree-shaped Program API. - provider/lowerer use CatalogEntry accessors (params(), param_count(), param_default()) and construct rules through Rule::new + field mutation (Rule is #[non_exhaustive]). - signature.rs rebinds source names the canonical catalog renamed or split: forcePlayerHero -> startForcingHero, stopForcingHero -> stopForcingCurrentHero, forceThrottle -> startForcingThrottle, stopChasingVariable -> stopChasing{Global,Player}Variable by resolved target kind. The old player stop-chase rejection is removed: the released catalog now carries stopChasingPlayerVariable and the emitter still normalizes through the PlayerVariable arg shape. - reconstruct.rs is rewritten from arena-id traversal to direct tree recursion. The linear action stream lets the classifier now reject what the arena model could not express: player/filtered events (previously a latent unreachable!), disabled actions and conditions, unbalanced If/ElseIf/Else/While/For/End streams, and dangling subroutine calls. - support-boundary.json declares the new rejection kinds plus the RemoveFromArray{ByValue,ByIndex} modify-op split; fixtures are respelled to current canonical action names. - limitations.md/support-matrix drop the obsolete player stop-chase gap claim.
… the canonical catalog Refs #107. The migration renamed four source-to-canonical bindings; this test fails loudly if a future catalog refresh drops or renames any bound action/value id, enum domain, or member id.
…er names Review follow-ups for #107: - ModifyPlayerVariable + AppendToArray reached assign_op_spelling's unreachable!() instead of returning a structured error; the modify emission is now shared between the global and player paths and emits `x.append(v)` on both (covered by a new surface-basic fixture action and a unit test). - check_names now validates what the manifest promises: names must be plain `[A-Za-z_][A-Za-z0-9_]*` non-keyword identifiers (workshop-rs accepts free-form phrases like `player score` that the native parser cannot re-read), must not collide with the `Event` pseudo-namespace, and must be unique across the global, player, and subroutine declaration tables. The keyword set moved to syntax::token so the lexer and the classifier share one source. - Duplicate `Subroutine`-event rules for one name are rejected instead of silently dropping the first rule's actions. - CallSubroutine targets are verified against declared subroutines (same class as the existing dangling value/event checks). - subroutine_rule_index is shared by the classifier and emitter; the manifest declares invalidName and drops the retired emptyName kind; stale WIR wording corrected to the public Program model.
…iases Review follow-ups for #107: - The arity cap rejected `Array(1, 2, 3)` because `array` is the catalog's one variadic entry with param_count 1; skip the cap for variadic entries. - resolve_entry's `ChaseVariableAtRate` and `StopChasingVariable` arms were dead (both names resolve through BUILTIN_BINDINGS first) and the latter mapped to a canonical id the 1.4 catalog no longer carries. The remaining arms cover only unbound spellings.
Second review follow-ups for #107: - SetPlayerVariable and PlayerVariable value references now apply the same receiver check as ModifyPlayerVariable: a non-Event-Player receiver emits `(recv).name` forms the frontend cannot lower. The three sites share check_player_receiver; the manifest kind generalizes playerModifyReceiver to playerReceiver. - String literals and rule names containing `"`, `\`, newline, carriage return, or tab are rejected: the source implementation does not decode `\` escapes, so the previous escape_string output re-parsed to different text or not at all. escape_string is gone; the emitter writes classified-safe strings verbatim. The customString/format text argument is now checked like any other value. - Emitted kind spellings renamed name-collision to nameCollision to match the manifest; assignMember is now declared; the kind doc comment describes the static-vs-prefixed kind categories honestly. - Duplicate subroutine body-rule errors are emitted in sorted order so the error vector is deterministic.
The `reconstruct-dangling-subroutine` sites emitted kind `subroutine` while the manifest declares `danglingSubroutine` for that code.
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.
Refs #107.
Summary
workshop-rs=0.3.7 → 1.4: the private arena WIR is gone, so allWorkshop-facing code now builds and walks the public tree-shaped
ProgramAPI. This PR is stacked on #122 (uses the new real-projectand reconstruction tests as migration validation).
CatalogEntryaccessor methods(
params()/param_count()/param_default()/param_type()),Rule::new+ field mutation (non-exhaustive),is_variadic()honored in the arity cap.
or split —
startForcingHero,stopForcingCurrentHero,startForcingThrottle,stopChasing{Global,Player}Variablebyresolved target kind. The released catalog now carries
stopChasingPlayerVariable, so the old player stop-chase rejectionis gone and the gap claim is removed from limitations.md and the
support matrix.
recursion over the public model. The linear action stream lets the
classifier reject what the arena model could not express —
player/filtered events (previously a latent
unreachable!),disabled actions/conditions, unbalanced control flow, dangling
subroutine calls, duplicate body rules, non-identifier/keyword/duplicate
declaration names, non-Event-Player receivers, and strings the source
implementation cannot decode. The boundary manifest declares the new
rejection kinds;
escape_stringwas deleted since emitted stringsare now classified-safe.
Test plan
cargo test --workspace --all-targets— all 16 suites greencargo clippy --all-targets -- -D warningscleansupport --checkmatrix validemit → reparse) including new player-append fixture action
every_declared_binding_resolves_against_the_canonical_catalogpins all ~90 builtin bindings + enum domains against catalog
drift
fixed and re-reviewed to PASS, including a confirmed panic on
player-variable append and name/escape totality gaps