induktio / thinker

AI improvement patch for SMACX.
https://discord.gg/XdFuwWzzku
GNU General Public License v2.0
70 stars 10 forks source link

2.4 ignore_reactor_power #14

Closed tnevolin closed 3 years ago

tnevolin commented 3 years ago

It correctly conducts combat. All units fight as if they have fission reactor. When a unit incurs 10 damage it is destroyed. So the 10 last unit HP are actually counts.

It doesn't work in other areas. Examples below

The display does not reflect this. Unit health bar is still computed with actual reactor. Create singular scout and set its damage to 10. You'll see its health bar reduced by 1/4. If you fight any other unit it will be destroyed immediately as having max damage already.

The healing does not take this into account as well. Singular unit heals 4 point at a time as in vanilla while fission one heals 1. So from your modified combat perspective singular unit heals 4 times faster. At the same time unit outside of the base can heal only up to 80% which means 32 for singular. So in your combat version it actually gets 2 HP only.

I suggest to use existing for psi combat reactor power ignoring mechanics. It is pretty simple and does not require a huge array of ones and such massive patching in 50 places.

induktio commented 3 years ago

Those healing/health bar inconsistencies would certainly need some kind of a fix. In any case I would consider it a requirement for this feature to correctly display combat odds, otherwise it results in very confusing behaviour from the user perspective.

The patching method itself is pretty simple to understand since it writes only new pointers to those locations and won't change the code flow per se. So I don't see a big problem even if it has to patch many locations. But it could be changed to something else if we can then fix a) health bars, b) healing, c) combat odds display at the same time.

tnevolin commented 3 years ago

It seems you are underestimating the gravity of the situation. šŸ™‚

Search for references to UNIT_reactor and VEH_damage_taken. There will be hundreds of them. This is where vehicle HP computation is hardcoded. It is everywhere. Just mere thought of modifying it gives me creeps. This is not the way to go.

As you probably know, I have spent enormous amount of time dealing with reactors. Tried different approaches. And I wasn't alone. There were like hundreds of post from different people trying to review and analyze the problem. Current state is the product of multiple discussions and opinions combined. Feel free to review them again.

The currently used solution seems to be the easiest one of all I have tried. It does not modify HP and it does not introduce any new mechanics - just reused the one already in the game. I dare to say it is a silver bullet that delivered workable solution and put my mind to the rest. There are still fixes needed to be done around that (like odds and other stuff), of course but they are more local and iteratively manageable.

You are absolutely can follow my footsteps on that or even sidetrack any way you like. I am about 95% sure there is no more simple and elegant way to do that but I am not stopping your from experimenting. At the same time I would definitely be glad to share my knowledge and review different approaches if you need my help.

induktio commented 3 years ago

The issue here is that you didn't explain which patches are actually required to fix the odds display to reflect actual odds, as in both units are treated as having a fission reactor according to the vanilla formula. We don't need to implement any fancy percentage calculations, just have it displayed correctly in the GUI according to the vanilla mechanics. The code required for that seems to be somewhat more than just patching 2 locations in battle_fight_2 function. If that patch alone worked, that would be great, but it is not really the case.

induktio commented 3 years ago

I think we can live with incorrect odds display for now if it fixes the side effects elsewhere. I realized there were also bugs in Thinker's combat damage calculations, so I pushed a new version with the WTP patch. Do you think this version works fine other than the odds display?

tnevolin commented 3 years ago

Didn't test it yet. Do you have version number for it?

I don't understand the importance of odd display fix for the moment. Yes, it should be correct and need to be fixed if it is broken after reactor patch. No doubt. However, this issue reports just reactor patch problems those need to be addressed first. Once it works we can have a look at odds.

I also cannot tell you how to fix odds as I never did it in first place and I also don't know which way your reactor patch goes. We can discuss it on a forum, of course.

My odds display is a complete redo. I did not bother to fix anything just recomputed it from the ground up and also modified the UI. That probably won't help here.

induktio commented 3 years ago

Hmm? There's no special version numbers unless the commits are tagged as such.

What I'm saying is that people have been playing this game for 20 years and formed a certain mental image of how the odds actually behave in combat. In addition to that, it's a problem if the dialog box displays different numbers when the units should be identical in strength. You might want to redesign certain parts of that combat interface, but it's still in my opinion secondary to having a consistent user interface experience across the game.

tnevolin commented 3 years ago

I am terribly sorry but you lost me completely. I absolutely agree and never objected about odds statement above. Just cannot understand how this is relevant to this specific reactor patch issue?

Odds stuff seems to be broader topic. I would prefer to discuss it on a forum.

induktio commented 3 years ago

Well, "discussions" for this repo is available on the top bar if there's a topic that is not some specific bug/issue with the mod. I would prefer to keep these topics in one place.

tnevolin commented 3 years ago

Sure thing. I don't mind to keep it in either place.

induktio commented 3 years ago

So you pushed an update for the combat odds display, are everything required for that contained in commit fc9edc8d? It looks interesting at least, I'll investigate it further.

tnevolin commented 3 years ago

I believe so.

induktio commented 3 years ago

One potential issue I can see is that the new display requires an additional entry in script.txt. Is there some specific reason it's not included in xscript.txt? Anyway, I know at least some players of this mod use non-English version of the game, so then requiring them to overwrite the txt file would be undesired. I'm thinking of just adjusting the vanilla dialog box with the new odds.

tnevolin commented 3 years ago

I am no aware about xscript.txt. What is it for? Should I put it there too? šŸ˜•

Sure. By adjusting you mean just putting other numbers then?

How localization is made? Is it just translation in scripts?

induktio commented 3 years ago

For example, one player contributed German language translation of alphax.txt to the repo. Don't have any other translations at the moment. Maybe should even include it with the release version. There might be some compatibility problems with terran.exe if you put stuff in the wrong file but I haven't checked it so that's just speculation.

tnevolin commented 3 years ago

For my knowledge xscript.txt is not used anywhere in any expansion.

induktio commented 3 years ago

Not really sure what is the purpose of that file then. Anyway, I can see one simple way to modify the odds to be consistent while ignore_reactor_power is active. Intercept the battle_fight_2 > best_defender call, save the ids. Then intercept two parse_num calls and divide the numbers by the reactor value to get them to match the actual strength. Unless there's more special cases that needs to be taken into account.

tnevolin commented 3 years ago

That is exactly the last approach I took! šŸ˜€

Let me share what I know.

Vanilla mechanics

Program computes so called unit "strength" which is a combination of all attack/defense factors and which is later on is used to compute round winning probabilities. This final strength is displayed in combat panel (below, not in odds dialog) after all factors are combined.

Then it computes the HP in following way:

HP = 10 * reactor power - damage taken

This is also displayed in combat panel as "power".

Then it multiplies "power" by strength = and builds the ratio of these values for attacker and defender. Then simplifies the fraction and displays it as odds.

Known considerations and caveats

divide the numbers by the reactor value to get them to match the actual strength.

You need to divide by power (which is remaining HP), not the reactor power.

Artifact max HP is always 1 instead of 10 * reactor power. I don't know if it is the same for unarmored probe but it seems like that since odds against them are always enormous.

Psi combat

In psi combat reactor power is ignored by vanilla design already! This is done by assigning each unit firepower equal to opponent reactor power so each hit takes so much HP from victim equivalent to their reactor power. So you need to first determine if it is a psi combat and them compute their effective HP.

effective HP = HP / reactor power, **rounded up!!!!**

And then delete by effective HP to get strength.

Keep in mind that psi combat can be initiated by psi weapon and armor alike as well as hand maid psi weapon. I.e. unit may not even be a psi unit by itself.

induktio commented 3 years ago

Yeah I can see that description seems to match modifiedDisplayOdds function. The implementation can be simplified quite a lot when it can just exit after that parse_num(1, defenderOdds); line. I would change the artifact comparison to weapon_type == WPN_ALIEN_ARTIFACT because unit_plan may not be reliable. Otherwise should probably use that solution as it is, mainly just clean up the hooking process.

tnevolin commented 3 years ago

Oh. Sure. You don't need the rest of my computations if you just displaying vanilla odds.

RE: artifact. Makes sense. I didn't know there is an artifact weapon! šŸ˜²

tnevolin commented 3 years ago

I can close it unless you want to keep it as reminder for something you want to do in future.