Repository navigation
Conversation
SV AutoStory built six near-identical start/end dropdown databases by hand and repeated the tutorial/main-story index offset maths in six helpers and two descriptions. make_story_select_database() and story_index() in Pokemon/AutoStory/Pokemon_AutoStorySelect.h do each once, for other games' AutoStory to reuse, with their tests beside them. No behaviour change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AKYzhrMnPK61XHWEch4a6E
|
@jw098 will need to look at this one since I assume the expectation is that SV AutoStory would get its options refactored to use this. |
pa-devbot
left a comment
There was a problem hiding this comment.
AI review: Clean, behavior-preserving refactor. Verified on this branch: it builds and the full command-line suite is green (20,333 passed / 0 failed / 3 skipped), including the 6 new tests (Pokemon::AutoStoryIndex, Pokemon::AutoStorySelectDatabase).
What's good
make_story_select_database()slugs by whole-list index and readsname()off the entry, matching the oldstd::to_string(index_num)builders. Thei < list.size()clamp is a small robustness win over the oldall_segments[i]indexing. Templating onEntry(any type withname()) is what lets one helper serve both segments and checkpoints.story_index()collapses thefirst_main_story_index + idxoffset into one place; all fourget_*_index()now route through it, and the two checkpoint descriptions now call their getters. Equivalent to the original if/else for bothTUTORIALandMAIN_STORY(only two enum values, defaultTUTORIAL).- Tests hit the interesting cases: whole-list slugging, tutorial pass-through, main-story offset, and the end-past-list clamp.
One gap in the SV refactor — the tutorial/main-story offset is still inlined in the two segment description functions, which were left behind while the sibling checkpoint descriptions were moved to call their getters:
SerialPrograms/Source/VideoGames/PokemonSV/Programs/AutoStory/PokemonSV_AutoStory.cpp:1030start_segment_description().../PokemonSV_AutoStory.cpp:1040end_segment_description()
For symmetry with start_checkpoint_description()/end_checkpoint_description() (which now call get_start_checkpoint_index()/get_end_checkpoint_index()), these could just be:
std::string AutoStory::start_segment_description(){
return " " + ALL_AUTO_STORY_SEGMENT_LIST()[get_start_segment_index()]->start_text();
}
std::string AutoStory::end_segment_description(){
return " " + ALL_AUTO_STORY_SEGMENT_LIST()[get_end_segment_index()]->end_text();
}That removes the last two inline copies of the offset and makes story_index() the single source of truth. Behavior is unchanged.
(Aside: no other game has an AutoStory program yet, so SV is the only consumer today — good that the helpers already live in Pokemon/AutoStory/ in the Pokemon namespace, ready to be reused the moment another game ships one.)
start_segment_description() and end_segment_description() computed the tutorial/main-story segment offset inline. They now call get_start_segment_index() / get_end_segment_index(), like the checkpoint descriptions, which removes the last two inline copies of that offset (pa-devbot review on PokemonAutomation#1828). No behavior change: both getters return the same index for tutorial and main story. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AKYzhrMnPK61XHWEch4a6E
|
Overall, looks fine to me. My only comment is that we currently assume only 2 main sections: Tutorial + Main. But if this changes in the future, we'll need to refactor it to handle N sections. |
story_index() maps a tutorial or main-story position to the flat story list. Note that this assumes two consecutive sections, which is all SV AutoStory covers, and that a branching postgame or DLC needs another model. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AKYzhrMnPK61XHWEch4a6E
added a NOTE -- might tackle that as I do PostGame (Seveii islands) for FRLG |
|
You only need separate story sections if the automation needs to take a break. e.g. between the SV tutorial and main story, the user had to manually level up the leak Pokemon. If there are no breaks required, you don't need a separate story section. |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AKYzhrMnPK61XHWEch4a6E
|
Makes sense — sections are break points. Updated the NOTE: postgame/DLC append to the last section unless they need their own break. |
Takes the reviewed version of the shared select code from PokemonAutomation#1828: Pokemon_AutoStorySelect.h and its tests in Pokemon/AutoStory/Pokemon_AutoStorySelect_Tests.*. The older AutoStory index tests in PokemonSV_Tests.cpp are dropped; PokemonAutomation#1828's suite covers them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AKYzhrMnPK61XHWEch4a6E
…pstream queue, tips Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AKYzhrMnPK61XHWEch4a6E
Moves SV AutoStory's repeated start/end dropdown and story-index code into two shared helpers that other games' AutoStory can reuse.
make_story_select_database()builds a start/end dropdown from a slice of the segment or checkpoint list, slugged by the index in the whole list. It replaces six hand-built databases.story_index()does the tutorial/main-story offset once, instead of in six helpers and two descriptions.Pokemon/AutoStory/Pokemon_AutoStorySelect.h, with their tests beside it. No behaviour change.Testing
Pokemon::AutoStoryIndexandPokemon::AutoStorySelectDatabase; fork CI passed on Mac, Windows Clang and Ubuntu.🤖 Generated with Claude Code