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 some problems of thievery #1582

Merged
merged 6 commits into from
Jun 21, 2022

Conversation

SaintCirno9
Copy link
Contributor

Summary

SUMMARY: [Bugfixes] "Fix some problems of thievery "

Purpose of change

Fix #1572
Fix #1581
Action "Sort out my loot" does not move items that do not belong to the player faction (no theft).

Describe the solution

For #1572, make the NPC talk about thievery immediately once they notice you steal something.
For #1581, move the code to right position (I believe they are placed wrong during the migration).
Add items owner check in move loot action.

Describe alternatives you've considered

Testing

Tested thievery in refugee center. Works as intended apparently.

Additional context

@SaintCirno9 SaintCirno9 force-pushed the fix-thievery-problems branch from 54cb088 to 79e3e13 Compare May 30, 2022 06:22
@SaintCirno9 SaintCirno9 force-pushed the fix-thievery-problems branch from 79e3e13 to 892e728 Compare May 30, 2022 06:28
@Coolthulhu Coolthulhu self-assigned this Jun 5, 2022
src/npc.h Outdated
@@ -1341,6 +1340,7 @@ class npc : public player
static constexpr tripoint_abs_omt no_goal_point{ tripoint_min };
job_data job;
time_point last_updated;
static bool has_thievery_witness;
Copy link
Member

Choose a reason for hiding this comment

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

static - now that's evil.
And unnecessary evil at that. You could inline npc::witness_thievery() where it is used and just handle all witnessing there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I‘ve noticed there are still some static variables in npc.h, so they are considered bad right?

@SaintCirno9 SaintCirno9 requested a review from Coolthulhu June 11, 2022 08:58
@Coolthulhu
Copy link
Member

Certainly looks better now, though I don't like it that item method sets player activity values.

Will have to spend some time to test it for edge cases, but if it works like it looks it does, it's good.

@Coolthulhu
Copy link
Member

Can't move-steal the items in advanced inventory at all. I assume this is intended.

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.

Problems of moving items that doesn't belong to the player faction npc dupe and crash
2 participants