Skip to content
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

Mask Sale Fix #2900

Merged
merged 4 commits into from
May 20, 2023
Merged

Conversation

Malkierian
Copy link
Contributor

@Malkierian Malkierian commented May 20, 2023

Prevent calling OnItemReceive for ITEM_SOLD_OUT, as there is no ItemTable entry and trying to find it in one would always error, and also no need I can think of for anyone to know about that item in the context of the hook.

Should fix mask sale crash on Switch.

Build Artifacts

…temTable entry and trying to find it in one would always error, and also no need I can think of for anyone to know about that item in the context of the hook.
@Malkierian Malkierian changed the title Prevent calling OnItemReceive for ITEM_SOLD_OUT, as there is no I… Mask Sale Fix May 20, 2023
soh/src/code/z_parameter.c Outdated Show resolved Hide resolved
…o `ITEM_NONE` to prevent mistyped return issues.
…returning ITEM_NONE to preserve expected autosave on mask sale while still fixing the crash. This will need to be implemented more cleanly later with story events.
@Malkierian
Copy link
Contributor Author

Malkierian commented May 20, 2023

Alright, this latest build does still fix it, and I feel it's the least intrusive way to handle the autosave given the fact it's intended to be temporary.

@Malkierian
Copy link
Contributor Author

Mainly, I just wanted to present this alternative before we went through with the ItemTable modification. We may yet decide that that needs to be done completely separate from the autosave issue.

Copy link
Contributor

@briaguya-ai briaguya-ai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works for me, let's make an issue for event-based autosave and note that we should be able to remove this on that issue

@leggettc18
Copy link
Contributor

Seems to have a merge conflict somewhere, on mobile so can't really tell where rn.

@briaguya-ai
Copy link
Contributor

@leggettc18 it was just the int vs uint thing, i went ahead and resolved it using the github web ui

@leggettc18
Copy link
Contributor

Figured that was probably it, I'll merge shortly

@leggettc18 leggettc18 merged commit 0b47a19 into HarbourMasters:develop-spock May 20, 2023
@Malkierian
Copy link
Contributor Author

Lol, I already made the issue for event-based saves.

@Malkierian Malkierian deleted the mask-sale-fix branch May 20, 2023 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants