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

Enhancement: MM-Style Static Explosion Radius #2322

Merged
merged 6 commits into from
Jan 21, 2023

Conversation

splemb
Copy link
Contributor

@splemb splemb commented Jan 1, 2023

In OoT, bomb explosions increase in size before despawning. In MM and OoT3D, this behaviour was changed so that bomb explosions have a fixed size, about halfway between the minimum and maximum sizes of OoT. This has the side effect of making bomb and bombchu hovering much easier to perform.

This PR adds a checkbox in Enhancements/Items to toggle between the original and modified explosion behaviour.

Comparision Of Explosion Collider Size With and Without Enhancement:
https://cdn.discordapp.com/attachments/818494239847088170/1059224147093360740/staticExplosions.mp4

Build Artifacts

Copy link
Contributor

@aMannus aMannus left a comment

Choose a reason for hiding this comment

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

Looks good overall, one small comment from me.

Comment on lines 180 to 181
this->explosionCollider.elements[0].dim.worldSphere.radius += this->actor.shape.rot.z + 8;
if (CVar_GetS32("gStaticExplosionRadius", 1)) {
this->explosionCollider.elements[0].dim.worldSphere.radius = 40;
Copy link
Contributor

Choose a reason for hiding this comment

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

The second number in CVar_GetS32 means it would default to being on which I don't really agree with, I'd change it to 0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, that was a typo on my part

@briaguya-ai briaguya-ai added the merge conflicts PR has conflicts that need to be resolved before it can be merged label Jan 19, 2023
@Archez
Copy link
Contributor

Archez commented Jan 21, 2023

@splemb can you please revert the libultraship update. Generally unless you changed something in libultraship and are needing to pull in a new commit, you should not commit the submodule change. If you are seeing libultraship is "changed" on your local changes, then you might need to run git submodule update to grab the expected commit for your branch's base.

@aMannus aMannus removed the merge conflicts PR has conflicts that need to be resolved before it can be merged label Jan 21, 2023
@briaguya-ai briaguya-ai merged commit 643a982 into HarbourMasters:develop Jan 21, 2023
@splemb splemb deleted the mmexplosions branch February 11, 2023 11:26
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

5 participants