search

HarbourMasters / Shipwright

3.22k stars 486 forks source link

Ice traps acting like treasure chests #814

Closed MrSanker closed 2 years ago

MrSanker commented 2 years ago

yeah so, looks like every single ice trap on the game acts like if you were opening a progress-item chest

also, in the video, the savefile is not randomized

https://user-images.githubusercontent.com/88207750/179368555-75873125-c0dc-4e54-9f95-80afba2e7a66.mp4

briaguya-ai commented 2 years ago

just tried to reproduce this, made sure i had fast chests turned off, started opening the ice trap chests in the light trial

i didn't see any long animations

you said "every single ice trap," could you try the ice traps in the room with 6 chests in the light trial?

leggettc18 commented 2 years ago

I am able to replicate this on a vanilla save with fast chests off. In both the ice trial (water trial?) and Light trial.

briaguya-ai commented 2 years ago

@leggettc18 just in rachael or can you get it to happen when running latest dev in debug from code too?

MrSanker commented 2 years ago

just tried to reproduce this, made sure i had fast chests turned off, started opening the ice trap chests in the light trial

i didn't see any long animations

you said "every single ice trap," could you try the ice traps in the room with 6 chests in the light trial?

oh hey, sorry for not reading earlier, but the awnser is yes, it happens here too, and in the ice cavern ones, and every single ice trap chest, also, just in case, this is rachael alfa, not a dev build

https://user-images.githubusercontent.com/88207750/179371560-75c2cbb3-152c-4fef-a867-45cd89d1e6d8.mp4

leggettc18 commented 2 years ago

@briaguya-ai I'm not sure if it's latest dev but it's one of the branches I was working on that has commits post Rachael Alfa. I'll pull latest dev in a minute. It looks to me like get GetItemEntry for the ice traps that was added for rando has CHEST_ANIMATION_LONG on it. The fix should probably be to not use that entry for vanilla rather than changing the chest animation though. Although that doesn't explain why you weren't able to replicate it if you're 100% certain fast chests was off for you.

EDIT: Confirmed it still happens for me on latest dev

briaguya-ai commented 2 years ago

weird, so i had fast chests on, then opened one, then turned fast chests off, opened another and wasn't able to repro

but after restarting the game with fast chests off from the get-go i was able to repro

MrSanker commented 2 years ago

weird, so i had fast chests on, then opened one, then turned fast chests off, opened another and wasn't able to repro

but after restarting the game with fast chests off from the get-go i was able to repro

sooooo, in order to reproduce the error you have to launch the game with fast chests off?

also one thing is that i tried to reproduce the error in the title screen savefile and it didnt worked, only works on real savefiles

briaguya-ai commented 2 years ago

looks like we're relying on getting the ogID from GetRandomizedItemId, which shouldn't be an issue but probably is in the ice trap scenario for some reason

as for the fast chests issue, that seems like a different issue, probably is fixed by reloading a room with fast chests off but i haven't done too much investigation there

now the debug save stuff is completely new, no idea how that would impact any of this...

leggettc18 commented 2 years ago

We're talking vanilla save files here, so GetRandomizedItemId wouldn't even be in the picture here would it?

briaguya-ai commented 2 years ago

it looks like we didn't have a 64dd check around this, but it shouldn't be a problem considering getrandomizeditemid would just be returning the ogid in that scenario, but even wrapping all that in a 64dd check didn't seem to fix it https://github.com/briaguya-ai/Shipwright/commit/6361f626b1256a9c735f247bd2a4bfdc48246b3f

leggettc18 commented 2 years ago

Actually, looking at it now, it's 100% because of the extra GetItemTable entry that was added for Ice Traps (assuming it was added so that Ice Traps that replaced freestanding items looked like Ice Arrows?)

    GET_ITEM(ITEM_ARROW_ICE, OBJECT_GI_M_ARROW, GID_ARROW_ICE, 0x3C, 0x80, CHEST_ANIM_LONG), // Ice Traps

This was added at index 124 or 0x7C, which used to be GET_ITEM_NONE before rando. GET_ITEM_NONE has a short chest animation, whereas this new fake Ice Arrows entry has a long chest animation. So replacing that with a short chest animation should be fine. That value isn't taken into account for the overworld freestanding item case in rando is it?

briaguya-ai commented 2 years ago

freestanding ice traps aren't rendering as ice arrows in rando, but ti'm pretty sure that's because of the itemIdToModel stuff in randomizer.cpp

i'm thinking adding the entry for ice traps was a way to make sure we could shuffle them in, and copying the ice arrow entry was probably just a quick way to add an entry there

so yeah, just changing that entry to use short seems like a good fix to me if it's tested and working

leggettc18 commented 2 years ago

Yep, the chest animations are short and the free standing ice trap (which looks like a huge rupee btw) doesn't seem to be affected by it. They didn't normally pop up a text box or anything did they?

briaguya-ai commented 2 years ago

no text box, and yeah, the huge rupee is from

{ GI_ICE_TRAP, GID_RUPEE_GOLD },

in itemIdToModel in randomizer.cpp

leggettc18 commented 2 years ago

Yup makes sense. I'm about to make a PR then.