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 Lua onItemCheck #5857

Merged
merged 1 commit into from
May 30, 2024
Merged

Fix Lua onItemCheck #5857

merged 1 commit into from
May 30, 2024

Conversation

The-Aerec
Copy link
Contributor

@The-Aerec The-Aerec commented May 30, 2024

onItemCheck was incorrectly returning the user in two parameters instead of the item in the second param slot

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.

This PR fixes onItemCheck to actually allow the item being used during the check to be referenced instead of the caster. The function is defined as:

OnItemCheck(CBaseEntity* PTarget, CItem* PItem, ITEMCHECK param, CBaseEntity* PCaster)

and was returning the following:

Target, Caster, param

it now returns:

result = onItemCheck(CLuaBaseEntity(PTarget), CLuaItem(PItem), static_cast<uint32>(param), caster)

Steps to test these changes

Reference item anywhere in onItemCheck

print(item) to see that you are indeed referencing an CLuaItem instead of CLuaBaseEntity

@zach2good
Copy link
Contributor

So caster is now unused?

@The-Aerec
Copy link
Contributor Author

The-Aerec commented May 30, 2024

So caster is now unused?

Yes, I can extend or modify the function to include the caster again. Or just change the place of the item return. Up to yall. Item was actually never being returned so you could never check anything about the item in onItemCheck

@The-Aerec
Copy link
Contributor Author

I have added caster to its proper place:
OnItemCheck(CBaseEntity* PTarget, CItem* PItem, ITEMCHECK param, CBaseEntity* PCaster)

auto result = onItemCheck(CLuaBaseEntity(PTarget), CLuaItem(PItem), static_cast<uint32>(param), caster)

@zach2good
Copy link
Contributor

There's code that expects the original layout (note the comma):
image

I agree with you about matching the C++ signature: std::tuple<int32, int32, int32> OnItemCheck(CBaseEntity* PTarget, CItem* PItem, ITEMCHECK param, CBaseEntity* PCaster)

So you'll need to change the existing onItemCheck usages, plus replace the original simplest version with the new extended version:

itemObject.onItemCheck = function(target, player)
and
itemObject.onItemCheck = function(target)

to

itemObject.onItemCheck = function(target, item, param, caster)

and make sure everything matches up in the itemObject.onItemCheck = function(target, player) case etc.

onItemCheck was incorrectly returning the user in two parameters instead of the item in the second param slot
@The-Aerec
Copy link
Contributor Author

Zachs request has been compeleted:
Please note I can't test every item in the game!

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.

I checked ccb_polymer_pump, all looks fine 👍

In the future, typically, you'd make the smaller logical changes or more sensitive changes in their own commit, and then the huge 2000+ file find/replace changes in another commit so things are more easily reviewable.

@claywar claywar merged commit a400d72 into LandSandBoat:base May 30, 2024
11 checks passed
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