UFO:Alien Invasion
Development => Newbie Coding => Topic started 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:
- having a limit to what a soldier can carry (also based in strength) so soldiers wont be sitting ducks crushed under the weight of their equipment (that happened once while testing - I'm not using sensible weights for testing so no surprise there)
- Actually changing the scripts (I'm not submiting any changes to them here) - sugestions for sensible weights and/or TU penalty welcome
- This should be shown to the user (or so I think) - sugestions and help on how/where to do this welcome (not good at UI desing)
- edit: almost forget: a way to prevent auto equiped actors (skirmish, aliens in campaign ...) from becoming overloaded/underequiped
I´d like to know your opinions/sugestions/help on this
-
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 :)
-
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
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)
-
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.
-
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 ?
-
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?
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
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
-
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.
-
I like more first concept (penalty for walking).
-
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...
-
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.
-
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 ?
-
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
-
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.)
-
-- 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%.
-
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...
-
cheater, cheater!!
-
;D good one, have you read my signature below?
-
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
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...
@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
-
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.
-
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.
-
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
not very elegant tough.
it was an understatment ;D
edit: forgot to attach diff and changed subject
-
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).
-
How should i credit you? any name and mail?
-
spamtome [dot] 10 [at] gmail [dot] com
not really needed to credit me you know
-
I apply it to the master. Thanks again for your contribution.
-
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 ?
-
@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)
-
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