Opened 10 years ago

Last modified 8 years ago

#2732 closed enhancement

[PATCH] Looting carried ressources and handling of garrisoned entities — at Version 3

Reported by: Radagast Owned by: Radagast
Priority: Should Have Milestone: Alpha 20
Component: UI & Simulation Keywords: patch
Cc: Patch:

Description (last modified by Radagast)

Both the loot defined in the simulation template and traded goods if the to be looted entity is a Trader, are already transfered.

This patch adds the following other inventory related transfers:

  • All carried resources are transfered to the looting entity. e.g. if the dead entity did carry some food, the food will be yours.

To achieve this, 2 distinct algorithms are given: 1) Directly add the whole carried resources' amount to the looting unit's player's resources. 2) Transfer all the carried resources' amount that the looting unit can carry.

  • Garrisoned entities are first being ejected, then the Ownership is transfered from the previous owner (enemy or ally thereof) to the looting unit's player, finally the unit is garrisoned inside the looting unit.

Edit as I'm always recognized as spam despite captcha. @Itms: Thank you Itms for the review, I agree.

I added the review keyword too early. I currently try to learn how to best handle those versions and keeping track of the changes. To get some feedback I submitted this patch. And I got what I wanted. Thanks a lot.

Once I have less trigger work and can resolve a strange git branching issue that somehow reloads my Looter.js twice (or perhaps I have to clear the cache), I will submit new versions (I have maintain a branch for each of my planned patches).

Next I will split the two approaches and make it committable (removing all but the most needed comments).

Change History (5)

comment:1 by Stan, 10 years ago

love it :)

by Radagast, 10 years ago

Fix syntax error due to double termination bracket. Reuse GarrisonHolder's function AllowedToGarrison to simplify. Fix wrong indentation (not using tabs).

comment:2 by Itms, 10 years ago

Keywords: review removed
Summary: [Patch] Extend the Looter component to transfer any carried resources and garrisoned entities (aka Inventory transfer).[PATCH] Looting carried ressources and handling of garrisoned entities

I didn't test the patch but regarding the style, a patch should be eventually committable. The code you write is hopefully made to be added to the game, and is not aimed at discussing design.

So you just can't leave all these commented out lines and explanations in the patch. On top of what I pointed out, they make the patch a pain to review. :(

Given that you have two distinct approaches, you should for instance make two patches, each one being "committable", not referencing the other one, etc.

Looking forward to the next version(s), thanks for proposing this code! :)

comment:3 by Radagast, 10 years ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.