weixin_39996478
weixin_39996478
2021-01-12 01:46

[RDY] Rewrite drop code, resolve some issues and implement the "backpack logic"

Rationale: make drop code simpler and more maintainable. The current implementation does rather simple thing by tangled means using wrong indirection. I'm gonna mitigate the complexity so that issues (#14035 is one of them) could be resolved without ugly workarounds.

~~This PR is pretty much WIP (just a start actually) so it's too early to criticise the intermediate changes, but I want to share the intent. It's still WIP, but suggestions are welcome.~~

EDIT:

Fixes #13126. Fixes #14035. Fixes #14346.

Implements some ideas from #16650.

Summary:

  • The code became much cleaner and [I hope] easier to comprehend / maintain. No more tangled conditions, passing big numbers of arguments, "dummy" arguments, copying where references are possible, big functions with non-obvious side effects, e.t.c.
  • There were three code paths of dropping (x2 via placing items directly on the map; x1 via the drop/stash activity). Now there's the single path for the player - through the activity. Even NPCs use it, but support of the activity for them is a bit hackish (same as for pulping corpses). Also, there's still a function for NPCs to drop items directly which I didn't touch for now. We probably should process activities for NPCs like we do for the player
  • There was a bug caused by that ambiguity: dropping an item using 'd'rop was consuming 250 moves, using multidrop - only 100. Now it's 100 in all cases. The drop cost will become calculable in future PRs
  • The "backpack logic" is now here and is working well (the implementation in the current master is broken and doesn't work). The new algorithm consists of the following steps:

1. Add a weapon (if it's held) to the resulting list 
using zero drop cost (you simply let go)
2. Sort chosen inventory items by volume in ascending order
3. Add missing dependent armor/clothes (e.g. additional powerarmor 
components if powerarmor itself is dropped)
4. Sort the selected worn items by storage in descending order, but so that 
the dependent items always go first
5. While chosen inventory items "fit" into the "freed" storage, add them to 
the resulting list using zero drop cost (they're "contained")
6. Add the following clothing to the resulting list and continue to the step 5
with the next one (if obvious conditions are met)
7. If player's volume capacity is exceeded, add the excess (random items) 
to the list using zero drop cost (no more the "Some items tumble to the ground." 
message. The items were in the container and shouldn't tumble independently)
8. If there's something left after excluding what was added on the step 7,
load that into the next activity and proceed, otherwise cancel the activity.
  • List of items reordered for dropping can be viewed in the DEBUG MODE. Here's an example (the power armor itself, the riffle and the camo tank tops were selected for dropping):

drop - I plan to show the drop costs (and the actual list of dropped items) in the multidrop dialog (in a further PR) - Volume recalculation was fixed (it's not linear because of the PACKMULE and DISORGANIZED traits. The previous code didn't take that into account). This will be useful for recalculating volumes without cloning g->u (in a further PR) - Dropped items are handled correctly: when dropped in a vehicle (after taking them off or if the storage capacity is exceeded) they stay in the vehicle when they can - The terrain where you drop items is described better (e.g. "pavement"/"grass"/"dirt" e.t.c instead of simply "ground" for any terrain).

Known issues: There are still player-oriented messages in the activity handlers, so when NPCs drop items, the game says that its you who drop them. I'll address that in a further PR.

该提问来源于开源项目:CleverRaven/Cataclysm-DDA

  • 点赞
  • 写回答
  • 关注问题
  • 收藏
  • 复制链接分享
  • 邀请回答

26条回答

  • weixin_39996478 weixin_39996478 4月前

    I'm not absolutely sure, but that probably isn't related to this PR. I didn't touch wear / pick-up code here.

    点赞 评论 复制链接分享
  • weixin_39719727 weixin_39719727 4月前

    Zlave item stashing is totally broken.

    点赞 评论 复制链接分享
  • weixin_39719727 weixin_39719727 4月前

    Also, has a tiny conflict now. Trivial, but still.

    点赞 评论 复制链接分享
  • weixin_39996478 weixin_39996478 4月前

    Zlave item stashing is totally broken.

    How exactly? I tried it just now and it worked fine.

    点赞 评论 复制链接分享
  • weixin_39719727 weixin_39719727 4月前

    I did this: - Spawn a hulk, kill it, enzlave it and rez it - Put a backpack on it - Try to drop items on it - Doesn't wait there, wanders off. Expected: gets the status, waits and receives items - Advanced inventory drop items on it - Takes noticeably less time than manual dropping - All items go on the floor instead of on the zlave - Wall it off so it can't run - Drop items on it - Items drop to the ground instead - 'e'xamine it and try to drop the items that way - Says that the items are too heavy and they drop to the floor

    点赞 评论 复制链接分享
  • weixin_39996478 weixin_39996478 4月前

    Well, dropping items on a zlave (via 'D'rop command) doesn't work in the master either. Stashing using 'D'rop makes perfect sense, but you request a new feature that just isn't there yet. Please don't say that something is "broken" when it works like it did.

    点赞 评论 复制链接分享
  • weixin_39719727 weixin_39719727 4月前

    Pretty sure it worked a while ago. I'll test it in master and check the differences. Still, all the other options are also broken.

    点赞 评论 复制链接分享
  • weixin_39996478 weixin_39996478 4月前

    I've fixed the last one (weight and volume were swapped). The rest seem to be fine.

    点赞 评论 复制链接分享
  • weixin_39719727 weixin_39719727 4月前

    OK, looks like master doesn't have drop-stash either.

    But master does have an effect preventing the zlave from moving when stashing stuff onto it.

    点赞 评论 复制链接分享
  • weixin_39719727 weixin_39719727 4月前

    Has a regression regarding zlaves, but those are a minor feature.

    点赞 评论 复制链接分享
  • weixin_39902472 weixin_39902472 4月前

    Can you outline what you've done? It's quite a lot of (unnecessary) work to reverse-engineer what you've changed and whether it's working as expected.

    点赞 评论 复制链接分享
  • weixin_39996478 weixin_39996478 4月前

    Sure, I'll write a detailed summary soon. There are a lot of changes. The PR is almost done (the "backpack logic" is what remains unfinished) and works fine. Some pieces of the information can be found in the commit descriptions.

    点赞 评论 复制链接分享
  • weixin_39751871 weixin_39751871 4月前

    Where's the summary for this?

    点赞 评论 复制链接分享
  • weixin_39996478 weixin_39996478 4月前

    I've just finished the code and 'll write the summary today, a few hours later. Sorry for the delay, was quite busy recently.

    点赞 评论 复制链接分享
  • weixin_39996478 weixin_39996478 4月前

    See the description.

    点赞 评论 复制链接分享
  • weixin_39996478 weixin_39996478 4月前

    Anything else to do before it's mergeable?

    点赞 评论 复制链接分享
  • weixin_39626162 weixin_39626162 4月前

    Let me read through this now

    点赞 评论 复制链接分享
  • weixin_39626162 weixin_39626162 4月前

    This is a lot of lines. Are there any smaller units it could be practically broken up into?

    点赞 评论 复制链接分享
  • weixin_39996478 weixin_39996478 4月前

    I don't think so. It compiles after each commit, but functions properly only as a whole. Its size still seems reasonable though.

    点赞 评论 复制链接分享
  • weixin_39626162 weixin_39626162 4月前

    Don't disagree it's just you're a lot more familiar with this code and I suspect that why this hasn't received it's fair share of attention.

    点赞 评论 复制链接分享
  • weixin_39996478 weixin_39996478 4月前

    Rebased.

    点赞 评论 复制链接分享
  • weixin_39719727 weixin_39719727 4月前

    NPCs told to put on an item will take the conflicting item off twice. It may be just a duplicated message.

    点赞 评论 复制链接分享
  • weixin_39719727 weixin_39719727 4月前

    Had this happen: - Spawn NPC - NPC was wearing a riot helmet - Gave him a top hat - He swapped the headwear and dropped the helmet - I picked up the helmet - Told NPC to wear the helmet - Debugmsg about top hat not being worn

    Possibly caused by the morale system and not here.

    点赞 评论 复制链接分享
  • weixin_39996478 weixin_39996478 4月前

    Possibly caused by the morale system and not here.

    That happens: could reproduce. Will look into that.

    点赞 评论 复制链接分享
  • weixin_39996478 weixin_39996478 4月前

    That was a bug I introduced myself lately (d729ca7346fc54e99583b5a3b60fe60ec6267607). See 7a719e2.

    点赞 评论 复制链接分享
  • weixin_39719727 weixin_39719727 4月前
    • Place some items on the ground
    • Place a wearable items on top of them
    • Open advanced inventory menu
    • Open worn items in one tab, items on the ground in the other
    • Move all from ground to worn
    • Asks whether to wield or wear (expected: wear silently)
    • Tries to pick up all the other items (a feature?), but if the inventory capacity is exceeded, it tries to pick them up normally (a minor bug)

    Not a huge problem, just a weirdness.

    点赞 评论 复制链接分享

相关推荐