Repository navigation
Lua hot reload: dispatch slots and closure owners named by what they name - #1555
Merged
Merged
Conversation
…red in the order of the groups
…ded implementation leaves as it is
…ctions of one signature are two groups
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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.
Two names in a hot-reload build depended on code elsewhere in the program, so an unrelated edit renamed them and the payload refused it. Both now come from what they name. Builds without
-luaHotReloadare unchanged.Dispatch slots
A dispatch slot (the class-table key a method call goes through) was named after its group's first member and then made unique against every name taken, in the order of the groups. Groups of one method name were numbered in the order of their classes' names: in Castle Fight every interface's
runhas its own slot,run,run1, ...run13(LayerScoperun10,DialogBodyrun4,ForceCallbackrun7). An interface or an implementation added whose class sorted before others renamed their slots, every closure bound to them lost its binding, and the edit was refused with "lost bindings which may remain stored (in class-table slot ...run10)", 128 times in one commit.In a hot-reload build a group is now named after its root, the first by key of the members none of the group's others override (an interface's
run, a base class's method), plus a hash of that root's key (LuaDispatchPreparation.rootOf). An implementation added to the group leaves the root as it is; naming after the first member by name did not, since a new class can sort first (addedInterfaceWithAMethodOfTheSameNameKeepsTheOthersSlots,addedImplementationSortingFirstKeepsTheSlot).Closure owners
A closure's identity includes its owner, which was built from the enclosing named scopes (package, classes) and the function's signature, but not its name: a function is no named scope. Two functions of one signature in one package gave their closures one identity, told apart by their order across both functions, so a closure added to the first moved the second's and was refused as a move (
closuresOfTwoFunctionsOfOneSignatureAreTwoGroups). The owner now includes the function's name.Evidence
Replaying Castle Fight's last 30 commits which change Wurst code: with the slot change alone 21 are accepted, up from 19 on master; both of the commits refused only for lost bindings now apply. The replay with both changes is running and follows in this description.
In the game, on the 3.0.1.24371 client, single player: the test map's suite with a thirteenth payload, PASS 65 of 65. The map made two closures of an interface
MatrixZeta(run) and two from functions of one signature at the start; the payload adds an interfaceMatrixAlphawith arun, whose class sorts first, and appends a closure in the first of those functions. The closures made at the start still dispatch to their own bodies (31 + 32 through the interface's slot, 41 and 42), and the new ones run (51 + 52). Both edits were refused before.Unit tests, Windows:
LuaHotReloadTests(124),LuaBackendAuditTests(300),LuaTranslationTests(152),ClosureTests(68),DeterministicChecks(38),DispatchImplementationTests(8),HotReloadPipelineTests(6). Each new test fails before its change. Linux is covered by PR CI; macOS is unverified.Not covered