UFO:Alien Invasion

Development => Newbie Coding => Topic started by: DarkRain on August 26, 2010, 12:50:12 am

Title: Encumbrance/TU penalty
Post by: DarkRain on August 26, 2010, 12:50:12 am
Hello

I wanted to have some opinions om my third experiment with the source, this time id about TU penaltys and encumbrance:

The purpose is to add a TU penalty to some items like armour and heavy weapons, the way I made this is apply the penalty to movement costs, this will make a soldier with say a machinegun have less mobility but be able to use max TUs to fire (assuming he/she is not encumbered) as opossed to have TUs cut from the max

Also to add weight to items so soldiers that carries a weight over a threshold (based in strength) will have their TUs cut during combat

Note that this is WIP missing features include:

I´d like to know your opinions/sugestions/help on this
Title: Re: Encumbrance/TU penalty
Post by: Duke on August 26, 2010, 02:04:28 am
The purpose is to add a TU penalty to some items like armour and heavy weapons, the way I made this is apply the penalty to movement costs, this will make a soldier with say a machinegun have less mobility but be able to use max TUs to fire (assuming he/she is not encumbered) as opossed to have TUs cut from the max
I fear I have not fully understood that concept. Please elaborate on that. TUs are TUs.

Looking at the diff, I'd like to see your concept (as I understand it) better encapsulated in some GetTUsAvailableForMovement() further down the call tree.

Thx for helping us anyway :)
Title: Re: Encumbrance/TU penalty
Post by: DarkRain on August 26, 2010, 02:20:15 am
I fear I have not fully understood that concept. Please elaborate on that. TUs are TUs.
Well english is not my first lenguage... I'll try to explain what I meant

The idea was to add a property to the objectdef (the TU penalty) for some items like heavy weapons that will increase the amount of TUs the actor needs to move, thus reducing his/her mobility (will spend more TUs for moving) but won't affect TUs given at the start of the turn, unlike encumbrance the other concept introduced wich will cut TUs given to the actor at the start of the turn based on strenght to weight of the items (also a new property) ratio

Quote
Looking at the diff, I'd like to see your concept (as I understand it) better encapsulated in some GetTUsAvailableForMovement() further down the call tree.

As I said this is not near finished yet, I'm open to sugestions

edit: maybe an example will help to explain: apply the attached diff over the one in the first post and

TU penalty: equiping the Combat Armour or the Rocket Launcher will cause a soldier to use 3 TU per square when walking and 4 when crouch walking (equiping both should cause them to use 4 and 6)

Encumbrance: equiping both the Combat Armour and the Rocket Launcher may cause (depending on strenght) the soldier to start the turn with less TUs than normal (maybe even 0 that part is still half done)
Title: Re: Encumbrance/TU penalty
Post by: Mattn on August 26, 2010, 09:41:13 am
well, in general i like the idea - having a soldier equipped with a heavy machinegun is not running as wide as a soldier equipped with a knife - but please don't put the knowledge of this down into the grid_* functions or the interface - try to encapsulate that a little bit better.
Title: Re: Encumbrance/TU penalty
Post by: Duke on August 26, 2010, 04:53:28 pm
I think I understood now: There are two concepts, 'penalty for walking only' and 'reduce max TUs', and your diff implements them *both*. Correct ?

I'd suggest the penalty to be expressed as a float factor like the crouching penalty.

I forgot that Grid_MoveCalc doesn't know the entity anymore. So encapsulating further down the calltree is not an option. Perhaps it's time to create some ActorMoveInfo_t and pass that instead of a dozen params.

Iterating the whole inventory is a little time-consuming. Grid_MoveCalc is called pretty often, especially in the AI code. Maybe you find a place to do that calculation further up the calltree ?
Title: Re: Encumbrance/TU penalty
Post by: DarkRain on August 26, 2010, 06:26:16 pm
I think I understood now: There are two concepts, 'penalty for walking only' and 'reduce max TUs', and your diff implements them *both*. Correct ?
That's right, maybe I should split it in two to keep things clearer?

Quote
I'd suggest the penalty to be expressed as a float factor like the crouching penalty.
Yes after some testing that seems the right thing to do

Quote
I forgot that Grid_MoveCalc doesn't know the entity anymore. So encapsulating further down the calltree is not an option. Perhaps it's time to create some ActorMoveInfo_t and pass that instead of a dozen params.

Iterating the whole inventory is a little time-consuming. Grid_MoveCalc is called pretty often, especially in the AI code. Maybe you find a place to do that calculation further up the calltree ?
those are good points
Title: Re: Encumbrance/TU penalty
Post by: Duke on August 26, 2010, 10:23:25 pm
That's right, maybe I should split it in two to keep things clearer?
I don't think so. As they handle similar problems, they should be implemented side by side.
I just had some problems in understanding because I thought of them as concurrent concepts.
Title: Re: Encumbrance/TU penalty
Post by: Kildor on August 29, 2010, 08:16:33 am
I like more first concept (penalty for walking).
Title: Re: Encumbrance/TU penalty
Post by: Duke on August 30, 2010, 12:34:44 am
Kildor, imho both concepts make sense at the same time:
- walking penalty for the weight (in eg. backpack)
- TUmax reduction for eg. light, but bulky armour

If users will understand that (by intuition !) is another question...
Title: Re: Encumbrance/TU penalty
Post by: DarkRain on August 30, 2010, 11:23:32 pm
Ok, have been busy lately (and without internet connection for a couple days), but I'm trying with a new approach to this, still half done, but I´d like to hear your comments before I go any farther in this direction.

edit: Oops included thing I didn't mean.
Title: Re: Encumbrance/TU penalty
Post by: Duke on August 31, 2010, 12:16:24 am
Well, 'the code has the truth', but I'm currently not in the mood to guess everything from the code ;)
So could you please give us a brief description of what your new approach is ?
Title: Re: Encumbrance/TU penalty
Post by: DarkRain on August 31, 2010, 01:16:23 am
Hi,
sorry I had only time for a quick post -- had to go get something to eat -- not very different from first approach but I tought I would add the total weight and tu penalty of the items in inventory_t and update it only when the charater's inventory is changed (instead of calculating it every time they are used), well actually I'm updating it at CL_CharacterSkillAndScoreCvars and calling actor_updatecurrent in the onChange event for the container nodes (but it will only work if you use DND as auto placing seems to be calling the onChange event only for armour) but anyway that's the main idea
Title: Re: Encumbrance/TU penalty
Post by: keybounce on August 31, 2010, 02:10:47 am
Reducing TU's for having too much weight is a bad thing. Did you ever notice that in XCom, you could not aim your weapon if you had too much in your back pack? (Aimed shot was 80% TU, and just about any "too heavy" penalty dropped you below that.)
Title: Re: Encumbrance/TU penalty
Post by: Duke on August 31, 2010, 10:57:39 pm
-- had to go get something to eat --
No problem. That's always prio 1. We can't use starved devs ;)

So the plan is to maintain the weight. That's good. Provided you can catch *all* the events that change it. Which can be difficult as you already noticed.
Do you think you can provide a little patch that makes auto placing call onChange ?
Do you think you found all the events ?

@keybounce:
DarkRain's implementation will apply the penalties in the most gradual way possible. So it's not like overloaded ? TUs -= 20%.
Title: Re: Encumbrance/TU penalty
Post by: Thrashard96 on September 01, 2010, 02:55:38 pm
If anyone will add penalty TUs, i will reverse them for my goodness (darn translation website, there isn't a word i forgot in its database).

Anyways...
Title: Re: Encumbrance/TU penalty
Post by: Kildor on September 01, 2010, 03:36:57 pm
cheater, cheater!!
Title: Re: Encumbrance/TU penalty
Post by: Thrashard96 on September 01, 2010, 03:38:09 pm
;D good one, have you read my signature below?
Title: Re: Encumbrance/TU penalty
Post by: DarkRain on September 01, 2010, 07:50:46 pm
Do you think you can provide a little patch that makes auto placing call onChange ?
Well, Itried a little to work that out but it didn't work quite as expected, so I ended calling the update function right there, at least for now
Still that might be something worth checking as I'm not sure it was *only* an error on my side, as it seemed to work for all container nodes besides the backpack (where it crashed the game), will look at it later -- I'm in my other PC right now
Quote

Do you think you found all the events ?
Not yet, I'm aware of at least a couple situations that will throw things out of sync...

Quote
@keybounce:
DarkRain's implementation will apply the penalties in the most gradual way possible. So it's not like overloaded ? TUs -= 20%.
That's right I'm using an exponential function to calculate the penalty so it will progress slowly -- at the begining -- but if you seriously overload your soldiers they might end crushed under the weight of their equipment (but I'm talking of something like the double or more than they should carry) ;D
Title: Re: Encumbrance/TU penalty
Post by: DarkRain on September 06, 2010, 01:04:08 am
Do you think you can provide a little patch that makes auto placing call onChange ?
Maybe something like the attached patch could do the trick -- not very elegant tough.
Title: Re: Encumbrance/TU penalty
Post by: bayo on September 06, 2010, 11:06:39 am
Hi, i dont read the problematic, and i dont have the full code here, anyway.

Some comments:

* please move assignation like "target = csi.idHolster" outside of param. (cause i dislike interleave like that (to me, it is hard to read, i prefer more lines, its the same for assignation inside if...).

* move UI_GetContainerNodeByContainerID to ui_node_container.c and .h

* first UI_GetContainerNodeByContainerID param should be a parent node (or a window node). UI_GetContainerNodeByContainerID (const uiNode_t* const parent... then u can use it like that UI_GetNode(parent, "holster"), and like that UI_GetContainerNodeByContainerID(node->parent, ...). We assume all containers have the same parent, it can be a panel (not only a window). Anyway it can be a deeper search, cause we dont call a lot this function.

* use a switch instead of the "if"s for UI_GetContainerNodeByContainerID. Better, check UI_ContainerNodeLoaded, we should use something like INVSH_GetInventoryDefinitionByID (namebyid, this name is hard coded somewhere (or in a script), we should not hardcode it another time). Also, "equip" is a little special (see UI_ContainerNodeLoaded), but battlescape only only use "equip" without sufix ATM. See the last note.

* Just a note, baseinventory is only used outside battlescape. And ATM only "equip_" is used outside the battlescape.

Anyway, sorry, the container code is very very bad ATM, and hard to clean.
Title: Re: onChange for autoplace
Post by: DarkRain on September 06, 2010, 10:40:55 pm
Here's a new update on the onChange subject (wich I'm plannig to use for the encumbrance/TU penalty), this is a bit better than before

Anyway, sorry, the container code is very very bad ATM, and hard to clean.
Well when I said
Quote
not very elegant tough.
it was an understatment ;D

edit: forgot to attach diff and changed subject
Title: Re: Encumbrance/TU penalty
Post by: bayo on September 07, 2010, 11:35:56 am
ammoNode = UI_GetNode(node->root, "equip_ammo")
i think ammoNode can be null. In this case it will crash. We also should protect null targetNode.

I can apply this current patch, and rework it a little. I dislike the way it work with "equip_ammo", anyway its greater than the previous code. Thanks a lot anyway, it's a good improvment for behaviour consistency.

One more time, i only talk about the node container code. Cause, the onChange event and everything from the UI script should not manage rules like that. That's the job of the inventory code (and all the crappy thing around, which anyway need, IMO, a deep refactoring).
Title: Re: Encumbrance/TU penalty
Post by: bayo on September 07, 2010, 07:31:06 pm
How should i credit you? any name and mail?
Title: Re: Encumbrance/TU penalty
Post by: DarkRain on September 07, 2010, 08:31:44 pm
spamtome [dot] 10 [at] gmail [dot] com


not really needed to credit me you know
Title: Re: Encumbrance/TU penalty
Post by: bayo on September 07, 2010, 09:34:34 pm
I apply it to the master. Thanks again for your contribution.
Title: Re: Encumbrance/TU penalty
Post by: Duke on September 13, 2010, 10:19:07 pm
I apply it to the master. Thanks again for your contribution.
Yeah thx, but I suppose that's just the *beginning* of DarkRain's contributions ;)

@DarkRain:
Did you already manage to jump on the GIT train ?
Title: Re: Encumbrance/TU penalty
Post by: DarkRain on September 14, 2010, 03:37:17 am
@DarkRain:
Did you already manage to jump on the GIT train ?

Yes I did -- it took me the whole day but I managed it
(my internet conection has been limited to 512kbps since it failed -- more than two weeks ago now -- and the company just said 'We will investigate the case, if you dont receive an answer in one month please call again' when the report about the conection speed was filed)
Title: Re: Encumbrance/TU penalty
Post by: DarkRain on October 13, 2010, 05:30:05 am
It's been a while, I had some tech problems and then my internet connection died again. . .

Just so nobody thinks I just forgot about this I will attach here the output of 'git diff master encumbrance_work' in my local repo clone, it might take a while before I can get back to it tough. . .

Thing here untested as that was the next step before continuing with this