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

[RDY] Stop NPCs from stealing from your vehicles #22144

Merged
merged 11 commits into from Nov 5, 2017

Conversation

Projects
None yet
5 participants
@angoddu
Copy link
Contributor

commented Oct 13, 2017

I have added the ability to lock all cargo parts through the interact menu. Each cargo part can be individually locked. Intentionally made it part specific to make it harder to abuse the system.

This has been a long standing bug and I decided that it was worth trying to tackle. NPCs will no longer steal from your shopping cart so long as it is locked.

Let me know if anything should be change.

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Oct 13, 2017

"Locking" a shopping cart?

Let's just drop the NPC pickup from vehicles if it's such a problem.
Having to manually lock every cargo tile sounds terrible.

Alternatively, allow "claiming" a remembered vehicle. A number of claimed vehicles could be limited per overmap (remembered vehicles are visible from overmap level at the code side).
Same behavior could be made for camps: random NPCs would not touch anything on the map tile with a billboard and camp designated around it.

@angoddu

This comment has been minimized.

Copy link
Contributor Author

commented Oct 13, 2017

It would be rather easy to stop NPCs from looting cars. It would be a simple solution but may be too broad. Would it mess up any kind of quest or something?

@kevingranade

This comment has been minimized.

Copy link
Member

commented Oct 13, 2017

@angoddu

This comment has been minimized.

Copy link
Contributor Author

commented Oct 14, 2017

I was thinking that the COVERED flag could also make the cargo inaccessible to NPCs. If it is able to block all light then it can make it so NPCs can't see or take these items

@kevingranade

This comment has been minimized.

Copy link
Member

commented Oct 14, 2017

No... NPCs are people and they're just as capable of opening containers as the player is, COVERED doesn't cut it.

angoddu added some commits Oct 16, 2017

Removed player cargo locking. Made the following parts CARGO_LOCKING:…
… floor trunk, stow board, all hatches, trunk, box, wooden box, folding wooden box, and cargo space
@kevingranade

This comment has been minimized.

Copy link
Member

commented Oct 17, 2017

Build is failing, you didn't clean up all the manual lock/unlock changes and that's breaking compilation.
In general that code needs to be removed anyway.
The number of vehicle parts covered by this is still too high, stowboards, boxes, and cargo spaces don't necessarally have locks. I can see a box with a lock, but it needs to be a new vehicle part, not the default. Stowboards and cargo spaces are just shelves and racks, they can't lock.

Also this needs to be surfaced to the player somehow, that can stay as a TODO for the moment since some players clearly expect NPCs to be unable to take things anyway, but we need to communicate to the player that some cargo locations are safe from NPCs and others aren't.

@angoddu

This comment has been minimized.

Copy link
Contributor Author

commented Oct 18, 2017

Where would I add the TODO comment?

angoddu added some commits Oct 18, 2017

Removed extra code that was forgotten about. Removed CARGO_LOCKING fr…
…om stow boards, boxes, and cargo spaces. Added new boxes that have CARGO_LOCKING.
@angoddu

This comment has been minimized.

Copy link
Contributor Author

commented Oct 18, 2017

npcmove.cpp is on the astyle-blacklist so do I need to worry that it failed it's astyle check? I only changed a few lines. Also I have no idea why this monster-test.cpp failed. I didn't touch it

@kevingranade

This comment has been minimized.

Copy link
Member

commented Oct 18, 2017

Npcmove.cpp is no longer on the a style blacklist in the main branch.
Merge from or rebase on master, then run make astyle.
The monster test failure is an intermittent upstream failure and isn't your problem.

Don't worry about it too much though, I can easily fix it when merging.

"location" : "center",
"flags" : ["CARGO", "CARGO_LOCKING", "TOOL_SCREWDRIVER", "BOARDABLE", "COVERED"],
"breaks_into": "ig_vp_frame"
},{

This comment has been minimized.

Copy link
@Coolthulhu

Coolthulhu Oct 19, 2017

Contributor

Vehicle parts support "copy-from". Don't specify everything from scratch.
Check out data/json/vehicleparts/battery.json for an example.

This comment has been minimized.

Copy link
@angoddu

angoddu Oct 19, 2017

Author Contributor

Good point. I will do this

angoddu added some commits Oct 19, 2017

Merge remote-tracking branch 'origin/master' into npc-thief-fix
Resolved merge conflicts in npcmove.cpp by accepting the master code
@angoddu

This comment has been minimized.

Copy link
Contributor Author

commented Oct 19, 2017

Could someone re-run the travis test please?

@kevingranade

This comment has been minimized.

Copy link
Member

commented Oct 20, 2017

Realized something else about this, we're having a lock (and key, but I'm less concerned about that) appear out of nowhere when you craft a frame into a trunk or hatch. Hinges and a latch were already pushing it a bit, but now it's starting to get a bit silly.

In the end, this is mostly a bookkeeping issue since most frames are harvested from vehicles anyway, so presumably we're getting a stock of hinges, latches, handles, and locks from tearing down other vehicles, so we can call it "abstracted" for now, but this is something to track in the future that can expand vehicle crafting.

@angoddu

This comment has been minimized.

Copy link
Contributor Author

commented Oct 21, 2017

I like this idea. It would be an interesting addition to vehicles

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Oct 21, 2017

Now that I think about it, having to detach and reattach every vehicle part just to make it lockable sounds very tedious.
Would be better if those parts just had a "lock-installable" flag of some sort and there was a "lock" part.

@tinukedaya

This comment has been minimized.

Copy link
Contributor

commented Oct 21, 2017

Now that I think about it, having to detach and reattach every vehicle part just to make it lockable sounds very tedious.
Would be better if those parts just had a "lock-installable" flag of some sort and there was a "lock" part.

I like this. It would also solve the "worry" @kevingranade had about abstracting too much.

@angoddu

This comment has been minimized.

Copy link
Contributor Author

commented Oct 21, 2017

I could make a vehicle lock part and make the parts with the "CARGO_LOCKING" flag have a "LOCKABLE_CARGO" flag.

@angoddu angoddu changed the title [CR] Stop NPCs from stealing from your vehicles [WIP][CR] Stop NPCs from stealing from your vehicles Oct 23, 2017

@angoddu

This comment has been minimized.

Copy link
Contributor Author

commented Oct 23, 2017

I'm having an issue with my location flag. I have it coded like
"location" : "on_lockable_cargo",
but it is being installable anywhere. What am I missing with the locations? I thought that you could just have the location be the same as the flag with "on_" before it.

angoddu added some commits Oct 26, 2017

Updated JSON_FLAGS with new flags
Added documentation to CARGO_LOCKING and LOCKABLE_CARGO flags
@angoddu

This comment has been minimized.

Copy link
Contributor Author

commented Oct 27, 2017

I fixed the location error. I needed to add a condition to the vehicle::can_mount() method since location isn't flexible like I thought.

@angoddu angoddu changed the title [WIP][CR] Stop NPCs from stealing from your vehicles [RDY] Stop NPCs from stealing from your vehicles Oct 29, 2017

@cainiaowu

This comment has been minimized.

Copy link
Contributor

commented Nov 1, 2017

Jenkins, rebuild.

@kevingranade kevingranade merged commit d1d6d65 into CleverRaven:master Nov 5, 2017

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.003%) to 22.252%
Details
gorgon-ghprb Build finished.
Details

@cake-pie cake-pie referenced this pull request Aug 29, 2018

Merged

[Done] Sheet metal stats rebalance and recipe fixes #25210

4 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.