-
Notifications
You must be signed in to change notification settings - Fork 152
[Rando] Clock Shuffle + Logic #1275
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
* Close attempt at animating the clock, not perfect but a good start. * Git gud Clock
Co-Authored-By: Caladius <[email protected]>
Eblo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First wave of feedback with plenty of questions. I think you know this already, but it looks like there is strangely a crash in CollisionCheck_SetAC for the clocks. This only seems to happen for the Zora-rangable ones, which call that function. I'm not sure what that's about. Maybe the weirdness with how the animated drawfunc is handled causes it to mess with its collider?
BetterSoDT feedback.. might leave this for a potential v2.. can't come up with a good solution atm
Current behavior is to load the time you selected, then abort and load the next available time if that isn't one. Not great. I think the most painless route is to just check if the selected time is actually available, and if not, tell the player as much instead of charging ahead.
All that aside, while I didn't do a full run, the bits I did were a fairly interesting way of challenging my understanding of this schedule-driven game. I think this will be a fun shuffle for sure.
Eblo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With enemy drops now being merged, this will now need to account for enemies that are exclusive to certain times. My review on that PR should have you covered for a primer: https://github.com/HarbourMasters/2ship2harkinian/pull/1282/files#diff-69c6aa4d2342ae40aead6a9a0efd7dee2c7ed6788bf2f998c60e36fb769f1e7f
This will also need rebased on the latest develop.
|
Quick thought that just occurred to me is that the scarecrow is another way in the game to advance time. I just tried it in a seed where I had Night 2 and Night 3. Tried it on Night 2, which advanced to Day 3, which then panicked and moon crashed. I left the clock tower to see Dawn of the First Day, which then loaded Night 2. I wouldn't want to lose the scarecrow behavior, so maybe this can be patched to just advance to the next chunk, or imitate vanilla Night 3 behavior if you're already on your latest chunk. And now that I think about it, granny's stories are another thing to test. Edit: Upon testing, granny seems fine at least. |
Eblo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small feedback update. One last thing is that my recent enemy drops v2 PR changes how enemy drops are defined. I went ahead and commented any sort of time window logic for your convenience. I'll want to do a more thorough review of this after that enemy drops PR is merged and incorporated into this one, but I found no issues after the scarecrow fix. Clock Shuffle (name pending?) is looking to be in pretty good shape.
mm/2s2h/Rando/Logic/Logic.h
Outdated
| #define CAN_USE_DAY2_RAIN_BEAN \ | ||
| (HAS_ITEM(ITEM_MAGIC_BEANS) && (CLOCK_DAY2() || CAN_PLAY_SONG(STORMS) || \ | ||
| (HAS_BOTTLE && (CAN_ACCESS(SPRING_WATER) || CAN_ACCESS(HOT_SPRING_WATER))))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably simpler to define this as CAN_GROW_BEAN_PLANT or day 2 and bean:
| #define CAN_USE_DAY2_RAIN_BEAN \ | |
| (HAS_ITEM(ITEM_MAGIC_BEANS) && (CLOCK_DAY2() || CAN_PLAY_SONG(STORMS) || \ | |
| (HAS_BOTTLE && (CAN_ACCESS(SPRING_WATER) || CAN_ACCESS(HOT_SPRING_WATER))))) | |
| #define CAN_USE_DAY2_RAIN_BEAN (CAN_GROW_BEAN_PLANT || (HAS_ITEM(ITEM_MAGIC_BEANS) && CLOCK_DAY2())) |
When that happens, I'll go back and rebrand to "Shuffle Time" as discussed in discord. |
|
With recent merges, this will need to do the following with a rebase:
|
Eblo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Top notch job on this so far. In practice in gameplay, this whole thing seems to work splendidly and is just plain fun. My feedback is almost entirely concerned with how things are under the hood, with plenty of questions.
I think that a number of my comments from prior reviews got lost in the mix, as they have not been addressed while others have. I'd like you to carefully go through the entire PR files themselves for review comments. Certain views on GitHub might be only showing so much.
Again, bang up job here. This was no small feat.
| CHECK(RC_ENEMY_DROP_TEKTITE, CanKillEnemy(ACTOR_OBJ_SNOWBALL) && CanKillEnemy(ACTOR_EN_TITE) && (IS_DAY1() || IS_DAY3())), // Day 1 and 3 only | ||
| CHECK(RC_ENEMY_DROP_WOLFOS, CanKillEnemy(ACTOR_OBJ_SNOWBALL) && CanKillEnemy(ACTOR_EN_WF) && IS_DAY2()), // Day 2 only |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My comments may have been ambiguous. These enemies appear at night also, e.g. Day+Night 2 for the Wolfos:
| CHECK(RC_ENEMY_DROP_TEKTITE, CanKillEnemy(ACTOR_OBJ_SNOWBALL) && CanKillEnemy(ACTOR_EN_TITE) && (IS_DAY1() || IS_DAY3())), // Day 1 and 3 only | |
| CHECK(RC_ENEMY_DROP_WOLFOS, CanKillEnemy(ACTOR_OBJ_SNOWBALL) && CanKillEnemy(ACTOR_EN_WF) && IS_DAY2()), // Day 2 only | |
| CHECK(RC_ENEMY_DROP_TEKTITE, CanKillEnemy(ACTOR_OBJ_SNOWBALL) && CanKillEnemy(ACTOR_EN_TITE) && (FIRST_DAY() || THIRD_DAY())), // Day 1 and 3 only | |
| CHECK(RC_ENEMY_DROP_WOLFOS, CanKillEnemy(ACTOR_OBJ_SNOWBALL) && CanKillEnemy(ACTOR_EN_WF) && SECOND_DAY()), // Day 2 only |
| CHECK(RC_ENEMY_DROP_WOLFOS, CanKillEnemy(ACTOR_OBJ_SNOWBALL) && CanKillEnemy(ACTOR_EN_WF) && IS_DAY2()), // Day 2 only | ||
| CHECK(RC_ENEMY_DROP_SNAPPER, CanKillEnemy(ACTOR_OBJ_SNOWBALL) && CanKillEnemy(ACTOR_EN_KAME) && IS_DAY3()), // Day 3 only |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| CHECK(RC_ENEMY_DROP_WOLFOS, CanKillEnemy(ACTOR_OBJ_SNOWBALL) && CanKillEnemy(ACTOR_EN_WF) && IS_DAY2()), // Day 2 only | |
| CHECK(RC_ENEMY_DROP_SNAPPER, CanKillEnemy(ACTOR_OBJ_SNOWBALL) && CanKillEnemy(ACTOR_EN_KAME) && IS_DAY3()), // Day 3 only | |
| CHECK(RC_ENEMY_DROP_WOLFOS, CanKillEnemy(ACTOR_OBJ_SNOWBALL) && CanKillEnemy(ACTOR_EN_WF) && SECOND_DAY()), // Day 2 only | |
| CHECK(RC_ENEMY_DROP_SNAPPER, CanKillEnemy(ACTOR_OBJ_SNOWBALL) && CanKillEnemy(ACTOR_EN_KAME) && THIRD_DAY()), // Day 3 only |
| CHECK(RC_ENEMY_DROP_WOLFOS, CanKillEnemy(ACTOR_OBJ_SNOWBALL) && CanKillEnemy(ACTOR_EN_WF) && IS_DAY2()), // Day 2 only | ||
| CHECK(RC_ENEMY_DROP_SNAPPER, CanKillEnemy(ACTOR_OBJ_SNOWBALL) && CanKillEnemy(ACTOR_EN_KAME) && IS_DAY3()), // Day 3 only |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| CHECK(RC_ENEMY_DROP_WOLFOS, CanKillEnemy(ACTOR_OBJ_SNOWBALL) && CanKillEnemy(ACTOR_EN_WF) && IS_DAY2()), // Day 2 only | |
| CHECK(RC_ENEMY_DROP_SNAPPER, CanKillEnemy(ACTOR_OBJ_SNOWBALL) && CanKillEnemy(ACTOR_EN_KAME) && IS_DAY3()), // Day 3 only | |
| CHECK(RC_ENEMY_DROP_WOLFOS, CanKillEnemy(ACTOR_OBJ_SNOWBALL) && CanKillEnemy(ACTOR_EN_WF) && SECOND_DAY()), // Day 2 only | |
| CHECK(RC_ENEMY_DROP_SNAPPER, CanKillEnemy(ACTOR_OBJ_SNOWBALL) && CanKillEnemy(ACTOR_EN_KAME) && THIRD_DAY()), // Day 3 only |
| CHECK(RC_WOODS_OF_MYSTERY_GRASS_22, true), | ||
| CHECK(RC_WOODS_OF_MYSTERY_GRASS_23, true), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong grass checks got marked:
| CHECK(RC_WOODS_OF_MYSTERY_GRASS_22, FINAL_DAY()), | |
| CHECK(RC_WOODS_OF_MYSTERY_GRASS_23, FINAL_DAY()), |
| CHECK(RC_WOODS_OF_MYSTERY_GRASS_09, FINAL_DAY()), | ||
| CHECK(RC_WOODS_OF_MYSTERY_GRASS_10, FINAL_DAY()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| CHECK(RC_WOODS_OF_MYSTERY_GRASS_09, true), | |
| CHECK(RC_WOODS_OF_MYSTERY_GRASS_10, true), |
| } | ||
|
|
||
| // Add remaining (non-starting) time items to pool | ||
| // Only works in random mode - progressive mode items are added elsewhere |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clarify where that is here, so that anybody working with this knows where to look.
mm/2s2h/Rando/Types.h
Outdated
| RI_CLOCK_DAY_1, | ||
| RI_CLOCK_NIGHT_1, | ||
| RI_CLOCK_DAY_2, | ||
| RI_CLOCK_NIGHT_2, | ||
| RI_CLOCK_DAY_3, | ||
| RI_CLOCK_NIGHT_3, | ||
| RI_CLOCK_PROGRESSIVE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This list is in alphabetical order for the most part, so these RIs don't belong here.
On another note, the RI names are partially user-facing at least due to the save editor. Should these be renamed to RI_TIME_*?
| CHECK(RC_GORMAN_MILK_PURCHASE, CAN_AFFORD(RC_GORMAN_MILK_PURCHASE)), | ||
| CHECK(RC_GORMAN_TRACK_GARO_MASK, CAN_PLAY_SONG(EPONA)), | ||
| CHECK(RC_GORMAN_MILK_PURCHASE, CAN_AFFORD(RC_GORMAN_MILK_PURCHASE) && IS_DAY()), | ||
| CHECK(RC_GORMAN_TRACK_GARO_MASK, CAN_PLAY_SONG(EPONA) && CAN_AFFORD(RC_GORMAN_TRACK_GARO_MASK) && IS_DAY()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While the Gorman race does cost money, it's a flat 10 Rupee fee and not tied into a shop check with a price. It's not gated by wallet size.
| CHECK(RC_GORMAN_TRACK_GARO_MASK, CAN_PLAY_SONG(EPONA) && CAN_AFFORD(RC_GORMAN_TRACK_GARO_MASK) && IS_DAY()), | |
| CHECK(RC_GORMAN_TRACK_GARO_MASK, CAN_PLAY_SONG(EPONA) && IS_DAY()), |
| EVENT(RE_ACCESS_FISH, true), | ||
| EVENT(RE_ACCESS_BUGS, true), | ||
| EVENT(RE_SETUP_MEET_ANJU, HAS_ITEM(ITEM_MASK_KAFEIS_MASK) && BETWEEN(TIME_DAY1_PM_01_45, TIME_NIGHT1_PM_09_00)), | ||
| EVENT(RE_MEET_ANJU, RANDO_EVENTS[RE_SETUP_MEET_ANJU] && BETWEEN(TIME_NIGHT1_AM_12_00, TIME_DAY2_AM_06_00) && (Flags_GetRandoInf(RANDO_INF_OBTAINED_ROOM_KEY) || CAN_BE_DEKU)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be for the midnight meeting, which makes the event name confusing. Maybe just RE_ANJU_MIDNIGHT_MEETING?
Eblo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One miscellaneous thing is that my comment regarding CheckTrackerDrawLogicalList is still not addressed. That, and there are conflicts with the Central.cpp logic file.
This is really close. This review is trivial things. I'm pretty happy with this overall.
| RI_TIME_DAY_1, | ||
| RI_TIME_NIGHT_1, | ||
| RI_TIME_DAY_2, | ||
| RI_TIME_NIGHT_2, | ||
| RI_TIME_DAY_3, | ||
| RI_TIME_NIGHT_3, | ||
| RI_TIME_PROGRESSIVE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think of naming the time RIs like so, to keep them both alphabetical and also the same order as what you have? I'm not 100% sure myself:
| RI_TIME_DAY_1, | |
| RI_TIME_NIGHT_1, | |
| RI_TIME_DAY_2, | |
| RI_TIME_NIGHT_2, | |
| RI_TIME_DAY_3, | |
| RI_TIME_NIGHT_3, | |
| RI_TIME_PROGRESSIVE, | |
| RI_TIME_1_DAY, | |
| RI_TIME_1_NIGHT, | |
| RI_TIME_2_DAY, | |
| RI_TIME_2_NIGHT, | |
| RI_TIME_3_DAY, | |
| RI_TIME_3_NIGHT, | |
| RI_TIME_PROGRESSIVE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm indifferent, but I left it as is for now. Just let me know how ya wanna proceed. The other feedback is addressed though.
Turns the 3-day cycle into collectible items that you need to find before accessing different time periods.
How it works:
Settings:
Must Have
Logic, I think is fully implemented, but could use a few passesLogic needed for pots, crates, snowballs, you name it.Nice to Have
Animated clock item (Cal will work on this, thank you!)Make all "remaining hours" calculation based on obtained clock itemsAdd daytelops (This is complete on my local machine, gonna test more before pushing)Configurable final hours length (Default is 6 hours before moon crash)SoDT textbox to state the correct half-dayBetterSoDT feedbackNeeds Fixing
SkipSoTCutscenes conflicts causing wierd cascading miscalculated overcorrection"Queue Randomizer Item Gives" section of save editor should not be conditional on RO_CLOCK_SHUFFLEOn file load, should not correct for owl savesDeku hopping during a correction causes infinite loop of sinkingseed generation issues for french vanilla and glitchless when clock shuffle is disabledSkipSoTCutscene still conflicts on N3 and reverts back to start of N3Build Artifacts