-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Refactor and consolidate wield #79719
Draft
marilynias
wants to merge
33
commits into
CleverRaven:master
Choose a base branch
from
marilynias:refactor-wield
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
+276
−357
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…into refactor-wield
add_msg_if_player
ad8d6a3
to
94ac2c2
Compare
should be ready for first looks |
use correct wield for 'one of the bottles is wielded' test
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
Bionics
CBM (Compact Bionic Modules)
[C++]
Changes (can be) made in C++. Previously named `Code`
Character / World Generation
Issues and enhancements concerning stages of creating a character or a world
Code: Build
Issues regarding different builds and build environments
Code: Infrastructure / Style / Static Analysis
Code internal infrastructure and style
Code: Tests
Measurement, self-control, statistics, balancing.
Crafting / Construction / Recipes
Includes: Uncrafting / Disassembling
EOC: Effects On Condition
Anything concerning Effects On Condition
Items: Ammo / Guns
Ammunition for all kinds of weapons and these weapons themselves
Items: Containers
Things that hold other things
json-styled
JSON lint passed, label assigned by github actions
Mechanics: Enchantments / Spells
Enchantments and spells
Melee
Melee weapons, tactics, techniques, reach attack
Missions
Quests and missions
NPC / Factions
NPCs, AI, Speech, Factions, Ownership
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Infrastructure "Refactor wield"
Purpose of change
wield() would clear the old item if it was in you inventory, but not if it came from the world.
Additionally there are a couple different wield() functions:
Cataclysm-DDA/src/game.cpp
Line 10423 in 27152a9
Cataclysm-DDA/src/game.cpp
Line 10527 in 27152a9
Cataclysm-DDA/src/avatar.cpp
Line 1377 in 27152a9
Cataclysm-DDA/src/npc.cpp
Line 1516 in 27152a9
I want to unify them.
some context: discord and #79716
Describe the solution
create npc::wield( item_location loc ) that takes the location, so I can remove the old location and wield_new( item it ) since sometimes you want to wield a newly created item without a location.
There are some subtle differences between the current wield function, which have to be implemented .
use item_location where available, for newly created items make wield_new method
Describe alternatives you've considered
Testing
compiles
all [w]ield functions work
trading npc resulting in wield works
wielding from reloading works
dropping into vehicle from [w]ield works without old game::wield logic
Some wield-times seem to complain.
Additional context
Things WILL break.