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

Character::i_add returns an item_location #58191

Merged
merged 1 commit into from Jun 19, 2022

Conversation

mqrause
Copy link
Contributor

@mqrause mqrause commented Jun 5, 2022

Summary

Infrastructure "Character::i_add returns an item_location"

Purpose of change

Initiated as a fix for the errors that popped up in #58093
Handling this with item_location is also generally preferable to item pointers or references.

Describe the solution

  • adjust Character::i_add and everything around it
  • adjust everything in tests
  • fix a bug in Character::best_pocket that popped up with this (parent of returned item_location is ::item_in_container when it should be ::item_on_person

While I was there I also did some minor adjustments in the tests, but only in lines I had to change anyway or lines next to them. Mostly stuff like changing CHECK( x == true ) to CHECK( x ).

Describe alternatives you've considered

Maybe also have map::add_item return an item_location, but this turned out bigger than expected already, so maybe another time.

Testing

Picking stuff up and spawning it with debug menu seems to work properly. Had no test issues either.
Making an ammo belt from linkage on the ground works, although that always only loads one bullet. Probably linkage on the ground isn't properly registered as available for reloading, but that's a different bug in a different place and doesn't break anything either.

Additional context

Going through the tests showed me there are tons of methods that should also should be changed to accept item_locations as parameters instead of item * or item &. Basically everything that handles real, existing items. Wield, wear, reload, that kind of stuff. But that's not a project I'm going to tackle any time soon.

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Jun 5, 2022
@mqrause mqrause force-pushed the i_add_location branch 2 times, most recently from 2a42485 to a5dae4d Compare June 6, 2022 21:43
@github-actions github-actions bot added Code: Tests Measurement, self-control, statistics, balancing. Mechanics: Enchantments / Spells Enchantments and spells labels Jun 6, 2022
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jun 7, 2022
@github-actions github-actions bot added the Items: Containers Things that hold other things label Jun 8, 2022
@mqrause mqrause marked this pull request as ready for review June 8, 2022 05:56
@github-actions github-actions bot added BasicBuildPassed This PR builds correctly, label assigned by github actions and removed BasicBuildPassed This PR builds correctly, label assigned by github actions labels Jun 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style Code: Tests Measurement, self-control, statistics, balancing. Items: Containers Things that hold other things json-styled JSON lint passed, label assigned by github actions Mechanics: Enchantments / Spells Enchantments and spells
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants