Repository navigation
Conversation
Declare stable actor ids, reference them in starter parties, and offer source-preserving legacy migration during validation. Ignore private player settings in new projects and cover silent real-game startup.
Exercise the generated Console 0.5.0 project through muted New Game startup before and after explicit, source-preserving migration. Preserve the released fixture bytes and note the required future Editor dependency.
Remove automatic actor-migration planning from ordinary interactive validation when no missing actor ID or unresolved actor reference exists. Keep explicit migration and confirmed repairs for legacy references; exercise the real title alert on a released 0.5.0 project.
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a166d82257
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| try { | ||
| $workspace = ProjectWorkspace::fromProject($workingDirectory); | ||
| $pendingActors = ActorIdentityMigration::getPendingActors($workspace->actorDatabase); |
There was a problem hiding this comment.
In a normal checkout, including the CI workflow's locked composer install, composer.lock installs ichiloto/editor 0.5.1, while this commit's README explicitly notes that this version does not contain ActorIdentityMigration. Consequently every validate invocation reaches this call, catches the resulting class-not-found error as “The project could not be read,” and exits with failure before running ProjectValidator; the newly added Composer tests that invoke validation will fail as well. Update and lock the Editor dependency to a release containing these APIs, or retain a compatible fallback until that release exists.
Useful? React with 👍 / 👎.
Remove play refusal to launch when renderer preparation fails. Check declared source fingerprints without building, offer update, continue, or skip, and add renderer:update for requested builds. Reuse verified package installation and preserve the previous payload on failed builds.
… chain ichiloto.json records "format"; a project without one is format 0. Each format is one step class implementing ProjectUpgradeStepInterface (target version, a plan with one human line per pending change, and apply producing a report). The chain runs every step from the project's format to its target in order and records the new format after each step. A later format adds a step and never edits an earlier one. The existing save metadata upgrade becomes step 1, SaveMetadataStep, with unchanged behaviour and the --id option. LegacyProjectUpgrader is removed; its logic moved into SaveMetadataStep. The command needs no arguments. It lists each pending step's changes and asks to continue; --dry-run changes nothing; in a Git working tree it refuses to run over uncommitted changes unless --allow-dirty is given; it prints the follow-up items and writes them to ichiloto-upgrade-report.md. An up-to-date project reports that nothing is needed. BREAKING CHANGE: without a terminal, ichiloto upgrade now changes nothing unless --yes is given; it previously wrote the metadata immediately. Its old output lines are replaced by the plan and the report.
Format 2 makes a map cell two terminal columns wide. The step regroups every map layer and event layer by rewriting only the nowdoc body, never evaluating it: an odd-width row gains a trailing space and a two-column glyph starting halfway through a cell gets a space before it, then the result must read back through MapLayer::parseGrid. Field x coordinates are halved at their integer literal tokens (NPCs and wander areas, triggers, event spawn points and scripts, common events, cinematic cast, waypoints and finalizers, the new game start), classified from the engine's command schema, subject kinds, movement route shapes and the fields the loaders read; horizontal move route steps are halved and listed. Retired tiles2d crops are removed and TwoColumnCellsMigration is appended to the save compatibility chain in the manifest's own source. The report lists event cells with two markers and layers that no longer line up, then compares reachability instead of listing every cell that became solid: walkable ground is flood-filled from each map's entry points over the old per-column and the new per-cell collision, and event areas, NPCs, transfer triggers and arrivals that are no longer reached are named, with each cut-off region, a representative cell and the cell that most likely closed it. Anything it cannot identify is reported rather than guessed.
New projects record the current project format. The starter map and the generate:map default are whole two-column cells with `##` walls, their event layers are rows of blank cells, and the new game starts in cell 2, the cell holding the old column 4. MapScaffolder refuses a row that ends halfway through a cell before creating anything. The startup test upgrades each copy of the released 0.5.0 project (and the optional EpicQuest copy) before starting it, as an author would; the released fixture itself is unchanged.
Both commands ask the engine (ProjectFormat::assertSupported) whether it reads the project's recorded format and, when it does not, print the engine's own explanation, which points to `ichiloto upgrade`, and exit before launching the game or reading its data. Play fixtures in the renderer tests now record the current format. This narrows both commands: play no longer launches, and validate no longer checks, a project whose format differs from the engine's until it is upgraded.
Two-column map cells changed how the terminal plays, so project format 2 is withdrawn. Reverted: 18beccf (TwoColumnCellsStep, src/Upgrade/TwoColumnCells, its fixtures and tests) and the map content of 8d3c965. Removed: the format 2 upgrade step and its report sections, the two-column starter map and generate:map default, blank-cell event layers and MapScaffolder's refusal of a row ending halfway through a cell. Kept: the numbered format chain with SaveMetadataStep as format 1, play and validate refusing a project whose format differs from the engine's, and new projects recording the engine's current format (1). Scaffolded maps are 48 one-column cells wide with a spawn at x 4 again. BREAKING CHANGE: ichiloto upgrade no longer converts to two-column cells, and a project recording format 2 is refused as newer than the engine.
generate:map takes --kind, one of the project's tilesets in assets/Data/Tilesets, and writes it into the new map's data as its tileset. When it is omitted it asks for the kind interactively and fails without writing anything non-interactively; a kind the project does not have is refused. A project without tilesets still gets a map without a kind, and the output says so. Changed behaviour: in a project with tilesets, a non-interactive generate:map without --kind now fails instead of creating the map.
The Console tracks the Editor's develop branch, and through it the Engine's, from Packagist until the 0.6 release, with minimum-stability dev and prefer-stable, as the Game does. generate:map --kind needs the Engine's tilesets, which the released 0.5 Engine lacks. The lock resolves the Editor at 61ca458 and the Engine at 2e32b6f. Before the 0.6 release, set the Editor constraint back to a tagged version.
ichiloto battle takes --renderer and --gpui-renderer, or asks, through the same renderer selector as play, offers a renderer update the same way, and hands the choice to the arena through ICHILOTO_RENDERER, restoring the caller's value afterwards. A renderer with --runs is refused, since a simulation draws nothing. The renderer update offer moves out of play into RendererUpdateOffer, so both commands share it.
ichiloto battle takes one --member per party member (up to four), as Actor[:level][,Slot=item...], with actors and items by id or name; without it the party is the starting party. The setup is the engine's battle test setup, used for playing and for --runs alike: playing hands it to the arena, which builds a fresh party from it for every fight, and --runs builds its party from it. Anything the party cannot be built from (an unknown actor or item, a bad level, a slot the actor lacks, equipment that does not fit) is refused before any battle starts, every problem named. Removed: the console's own starting-party builder, which loaded actor files by name and bypassed the engine's actor identity resolution; the engine's setup replaces it. Also removed the README note calling ichiloto battle a placeholder, which was no longer true. A project whose engine has no battle test setup is told to update its engine.
There was no way to test a command, skill or summon before the game made it available. --member now takes Commands=, Skills= and Summons=, each a list separated by |: Commands replaces the member's command menu, by command id or the label the project shows for it; Skills grants abilities or spells from the project's skill catalogue; Summons grants summons by id. They resolve through the Engine's own command types, skill catalogue and summon library, and every unknown reference is named before a battle starts. The keys never become equipment slots. They reach the Engine's battle test setup for both playing and --runs. A project engine that cannot take them says so instead of ignoring them.
…ry problem --runs simulates with every battler attacking, so a member's Commands, Skills and Summons would never be used and its numbers would pretend otherwise. A simulation with a loadout is now refused, explaining that playing the fight uses them; levels and equipment still simulate. This narrows the previous commit, which passed loadouts to --runs. A member whose loadout does not resolve is no longer built, so every unknown command, skill and summon is reported together instead of the first empty list stopping the setup. The README example gives Ifrit to Kaelion, who may hold it. Tests cover the loadout reaching the setup by id and label, every unknown reference named, an ineligible summon holder refused by the Engine, and the --runs refusal.
`ichiloto edit --gui` launches the native editor window with this console's hidden `edit:host` as the session host it edits through (the Editor's SessionHost over standard input and output). The terminal editor is unchanged and remains the default. - GuiEditorLocator finds the executable without building or downloading it: ICHILOTO_GUI_EDITOR, else a release then a debug build in the gui-editor checkout beside the console; otherwise it says how to build it. - EditorProjectBootstrap owns preparing a process to edit a project (the engine and the project's PSR-4 classes); EditCommand's private bootstrap helpers moved there so both interfaces prepare it one way.
EditorProjectBootstrap sets ICHILOTO_CONSOLE_BIN to this console unless the author named one, so both the terminal and the graphical editor play test through the console that opened them rather than another found in the project or on the PATH.
New projects author enemies one record per file in assets/Data/Enemies, so enemies.php returns what EnemyCatalog loads from there instead of an empty list that would never include them.
…p loads A new project's skills.php is the barrel over assets/Data/Skills, as enemies.php is over Enemies; abilities.php and magic.php are no longer written. The battle test fixture authors its skills as records.
…ords that items.php loads
The key is checked against the battle presentation's arenas, naming them when it is unknown. --runs and the terminal renderer refuse an arena they would not draw.
On macOS the locator prefers gui-editor/build/macos/Ichiloto.app, whose executable links to the release build, before the bare release and debug builds.
Last Legend's Ifrit is now Djin, so the help and README examples name djin. The help example's member is Kaelion, since Liora never summons.
Without --member, ichiloto battle sets up the battle test its project keeps in system data (the Engine's ProjectBattleTest: its party, else the starting party, and its arena). --member replaces only the party and --arena only the arena. A malformed project battle test is refused with its problems. Engines without ProjectBattleTest keep the starting party.
Remove application of the develop-to-main restriction to repositories without an Andrew-created develop branch. Leave those repositories outside this remediation and do not infer authority to create develop or publish main. Preserve existing-develop integration, explicit branch-publication permissions, branch preservation, history safeguards, and current repository guards.
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
1 participant
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.
This pull request introduces several important enhancements and fixes related to actor identity migration, validation, and project scaffolding, along with improved error messaging and new test fixtures. The most significant update is the addition of the
--migrate-actor-idsoption to thevalidatecommand, enabling interactive and non-interactive migration of missing actor IDs and repair of legacy references. Additional improvements include more informative error handling for renderer manifest issues and updates to project scaffolding to explicitly assign actor IDs.Actor Identity Migration and Validation:
--migrate-actor-idsoption to thevalidatecommand, enabling both interactive and non-interactive migration of missing actor IDs and repair of legacy actor references. The command now reports pending migrations, prompts interactively when appropriate, and applies migrations with improved error handling and reporting. (src/Commands/ValidateCommand.php,README.md) [1] [2] [3] [4] [5]composer.jsonto include new validation and migration tests. (composer.json)Project Scaffolding and Actor Data:
src/Support/NewProjectScaffolder.php,src/Commands/GenerateActorCommand.php) [1] [2] [3] [4]Renderer Manifest Error Handling:
src/Support/RendererPackageInstaller.php) [1] [2]Test Fixtures:
released-console-0.5.0-projecttest fixture, including.gitignore, actor, system, and data files, to support compatibility and migration testing. (tests/fixtures/released-console-0.5.0-project/) [1] [2] [3] [4] [5] [6]These changes collectively improve the reliability and maintainability of actor identity management, enhance user feedback during validation and installation, and provide a stronger foundation for future migrations and project upgrades.
Git workflow governance
This update removes direct-main publishing allowances in repositories where Andrew created
develop, and removes universal workflow wording that imposed it on repositories without that branch. It installs mirrored agent/Claude/Copilot instructions, a workflow policy, PR template, and portable Git guards. Remote working branch publication, branch deletion and history rewriting retain their explicit permission requirements..github,docs, anddemo-repositoryhave no remotedevelopand are excluded; no branches were created there.The local guards reject commits/merge commits and pushes to main. The push guard accepts only authorized fast-forward updates from local develop to existing remote develop; it does not grant publishing permission. Existing hooks and work are preserved. Local hooks can be bypassed by APIs or direct ref operations, so they are one layer alongside repository instructions and available server enforcement.
Validation: 42 guard/installer checks in real temporary Git repositories and bare destinations; five PR source-validator cases; shell syntax and governance whitespace checks. No gameplay, asset or dependency changes are in the governance commits; Linux/native gameplay suites were not run. This develop-to-main PR also includes earlier queued work, which must be reviewed separately.
The trusted
pull_request_targetsource validator does not check out or execute PR code. It must first reach main through this PR before its status can be required. Main update and PR/history rules are active on the four public participating repositories: engine, editor, console and gpui-renderer. Private branch rules are unavailable on the current Free organization plan. No direct-main push, history rewriting, branch deletion, tag or release occurred.Governance/develop head:
e5f71fc208da4ecce32c5f4578d125f3b38dfe52.