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

[Fix] Souleater effect modifier stacking #4557

Merged
merged 2 commits into from
Nov 12, 2023

Conversation

Xaver-DaRed
Copy link
Contributor

@Xaver-DaRed Xaver-DaRed commented Sep 27, 2023

I affirm:

  • I understand that if I do not agree to the following points by completing the checkboxes my PR will be ignored.
  • I understand I should leave resolving conversations to the LandSandBoat team so that reviewers won't miss what was said.
  • I have read and understood the Contributing Guide and the Code of Conduct.
  • I have tested my code and the things my code has changed since the last commit in the PR and will test after any later commits.

What does this pull request do?

As per this BG article https://www.bg-wiki.com/ffxi/Souleater
Equipment that enhances Souleater (Souleater +X) does not stack. An exception happens with augments.

This PR (attempts to):

  • Correct this by using a very useful function found in lua_baseentity: getMaxGearMod() which returns the highest value of a modifier found in gear, insstead of the total value.
  • Implements that function in battleentity, so it can be used in core.

Steps to test these changes

Use Souleater with different pieces of gear that enhance Souleater and see if it only takes the higest value.

@Xaver-DaRed Xaver-DaRed changed the title [Fix] Souleater effect modier stacking [Fix] Souleater effect modifier stacking Sep 27, 2023
@Xaver-DaRed Xaver-DaRed force-pushed the WSoul branch 2 times, most recently from b2d9f56 to a741921 Compare September 28, 2023 00:07
Copy link
Contributor

@zach2good zach2good left a comment

Choose a reason for hiding this comment

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

Seems fine apart from those two points

}
}
return maxModValue;
return static_cast<CBattleEntity*>(m_PBaseEntity)->getMaxGearMod(static_cast<Mod>(modId));
Copy link
Contributor

Choose a reason for hiding this comment

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

Inside the function that this calls, you're checking for whether or not this is a Char entity, so why not cast to a char here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it to CCharEntity*

{
ShowWarning("CLuaBaseEntity::getMaxGearMod() - m_PBaseEntity is not a player.");
ShowWarning("Invalid Entity (NPC: %s) calling function.", m_PBaseEntity->GetName());
Copy link
Contributor

Choose a reason for hiding this comment

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

This could get called on a Pet or a Mob, not necessarily just an NPC

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it to Non-PC, since it is meant for player characters only.

@Xaver-DaRed Xaver-DaRed added enhancement New feature request and removed enhancement New feature request labels Nov 8, 2023
@TeoTwawki TeoTwawki merged commit cae8c47 into LandSandBoat:base Nov 12, 2023
13 checks passed
@Xaver-DaRed Xaver-DaRed deleted the WSoul branch January 12, 2024 03:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants