weixin_39820158
weixin_39820158
2021-01-12 01:40

cleanup quiver function

Before : The arrow calls item::add_ammo_to_quiver to check player's worn quiver, doing quiver insertion itself. Changed : Player calls player::add_ammo_to_worn_quiver, and each quiver calls item::quiver_store_arrow to store arrows.

Maybe it is more understandable.

TESTING [✔] Wear an empty quiver, pickup 10 arrows : quivered 10 arrows. [✔] Wear an empty quiver, pickup 25 arrows : quivered 20 arrows, 5 to inventory. [✔] Wear an empty large quiver, pickup 50 arrows : quivered 50 arrows. [✔] Wear an empty large quiver, pickup 65 arrows : quivered 60 arrows, 5 to inventory. [✔] Wear a full quiver, pickup 1 arrow : 1 to inventory.

[✔] Wear a quiver(1 carbon fiber arrow in) + large quiver(empty), pickup a 1 carbon fiber arrow : ~~quiver with carbon fiber arrow(2), But large quiver changed to 'large quiver with carbon fiber arrow'.. need to investigate.~~ Fixed. quiver with carbon fiber arrow (2), empty large quiver.

[✔] Wear a quiver(1 carbon fiber arrow in) + large quiver(empty), pickup a 1 flaming arrow : quiver with carbon fiver arrow(1), large quiver with flaming arrow(1) [✔] Wear a quiver(1 carbon fiber arrow in) + large quiver(1 flaming arrow in), pickup 1 plastic arrow 1 : 1 plastic arrow to inventory.

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

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

5条回答

  • weixin_39902472 weixin_39902472 4月前

    You say maybe it's more understandable, all I see is a bunch of code moving around and potentially breaking things. I need you to make a case for why this is an improvement.

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

    Thanks for advisory, I'll fix as you commented. And little question.. Is there way I can sort quiver using bool item::operator<(const item& other) const?

    It seems that item::add_ammo_to_quiver tends to do a many job at once. The function knows player and iterates it to get valid quivers. It knows all of worn quiver's capacity, and it manipulates player's moves. As a arrow, it knows little much about other objects.

    If we approach from the outside(player -> quiver -> arrow) player can check it's valid worn quiver, and reduce it's moves by each quiver function result. It doesn't care how quiver's store arrow. Each quiver can store arrow and return result, and they don't care how player's move change/other quiver's capacity state.

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

    Thanks for advisory, I'll fix as you commented. And little question.. Is there way I can sort quiver using bool item::operator<(const item& other) const?

    You could add a wrapper lambda, that takes the pointers that are to be compared, dereferences them and applies operator<:

     C++
    std::sort( begin, end, [](const item *a, const item *b) { return *a < *b; });
    

    ~~But you apparently don't want the logic from item::operator<, it compares by the item category first, than item type, than charges. You want to sort by the charges of the content.~~

    Actually, looking at the implementation of item::operator<, it might accidentally in this situation do what you want: all quivers should fall into the same category, sorting of quivers that contain different ammo types does not matter anyway and quivers containing the same ammo will be sorted according to the contained ammo count.

    However, having a dedicated sorting function / lambda which compares explicitly by the contained charges seems clearer.

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

    Sort quivers by dedicated sorting function item_compare_by_charges. Move cost of each quiver is min( 100, number of quivered * 10), as the old code intended.

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

    Thanks again for the code feedback.

    点赞 评论 复制链接分享

相关推荐