Project

Profile

Help

HostedRedmine.com has moved to the Planio platform. All logins and passwords remained the same. All users will be able to login and use Redmine just as before. Read more...

Feature #913055

Detailed Combat Info

Added by Charlie Galik 3 months ago. Updated 28 days ago.

Status:
Closed
Priority:
Normal
Category:
Server
Sprint/Milestone:
Start date:
2021-01-05
Due date:
% Done:

50%

Estimated time:

Description

It is desired that there be more information when there is a combat. Information like from the veterancy, HP before and after the attack, id numbers, attack and defense and firepowers calculated by the server, percent chance of winning the combat, and whether the enemy got upgraded.

Most of this information is presented to the client but I think it would be better to include a more detailed combat message from the server. This is especially helpful for learning what happened from an attack when you are not online from the attack, and have to figure out what happened from only the server messages. It also is more fun to see if you win a very low chance attack, and very frustrating to see if you lost an almost sure thing.

Another thing that would be helpful is displaying if there was a tired attack or not.

0001-Detailed-combat-info-from-server-message.patch (16.8 KB) 0001-Detailed-combat-info-from-server-message.patch Charlie Galik, 2021-01-05 09:16 PM
0001-Detailed-combat-info-from-server-message.patch (17.8 KB) 0001-Detailed-combat-info-from-server-message.patch Charlie Galik, 2021-01-08 07:33 AM
0001-Detailed-combat-info-from-server-message.patch (16.1 KB) 0001-Detailed-combat-info-from-server-message.patch Charlie Galik, 2021-01-14 07:45 AM
0001-Detailed-combat-info-from-server-message.patch (16.3 KB) 0001-Detailed-combat-info-from-server-message.patch Charlie Galik, 2021-01-15 08:19 AM
0001-Detailed-combat-info-from-server-message_S2_6.patch (16.2 KB) 0001-Detailed-combat-info-from-server-message_S2_6.patch Charlie Galik, 2021-01-20 08:41 PM
0001-Detailed-combat-info-from-server-message_S3_0.patch (16.3 KB) 0001-Detailed-combat-info-from-server-message_S3_0.patch Charlie Galik, 2021-01-20 08:41 PM
0001-Detailed-combat-info-from-server-message_S2_6.patch (16.1 KB) 0001-Detailed-combat-info-from-server-message_S2_6.patch Charlie Galik, 2021-01-21 11:33 PM
0001-Detailed-combat-info-from-server-message_master.patch (16.3 KB) 0001-Detailed-combat-info-from-server-message_master.patch Charlie Galik, 2021-01-21 11:33 PM
0001-Detailed-combat-info-from-server-message_S3_0.patch (16.3 KB) 0001-Detailed-combat-info-from-server-message_S3_0.patch Charlie Galik, 2021-01-21 11:33 PM
0001-Detailed-combat-info-from-server-message_S2_6.patch (16.2 KB) 0001-Detailed-combat-info-from-server-message_S2_6.patch Charlie Galik, 2021-01-26 08:47 PM
0001-Detailed-combat-info-from-server-message_S3_0.patch (16.4 KB) 0001-Detailed-combat-info-from-server-message_S3_0.patch Charlie Galik, 2021-01-26 08:51 PM
0001-Detailed-combat-info-from-server-message_master.patch (16.4 KB) 0001-Detailed-combat-info-from-server-message_master.patch Charlie Galik, 2021-01-26 08:51 PM

Related issues

Related to Freeciv - Feature #778499: Better indication of combat resultsNew

<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>

History

#1 Updated by Charlie Galik 3 months ago

Here's a patch file attached.

Much thanks to Louis Moureaux for the help.

Here's a test of it in action:

Combat: green Sumerian Warriors [id:135 attack:1 HP:10] has a 4.275% chance against the green Greek Warriors [id:165 defense:2 HP:10].
Combat result: Sumerian Warriors attack succeeded against the Greek Warriors. Sumerian Warriors lost 9 HP, has 1 HP remaining and achieved the rank of veteran.
Combat: green Sumerian Warriors [id:137 attack:1 HP:10] has a 4.275% chance against the green Greek Warriors [id:171 defense:2 HP:10].
Combat result: Sumerian Warriors attack failed against the Greek Warriors [lost 3 HP, 7 HP remaining and achieved the rank of veteran].
Combat: veteran Greek Warriors [id:166 attack:2 HP:10] has a 25.85% chance against the veteran Sumerian Legion [id:157 defense:4 HP:5].
Combat result: Greek Warriors attack failed against the Sumerian Legion [lost 4 HP, 1 HP remaining and achieved the rank of veteran].

#2 Updated by Marko Lindqvist 3 months ago

I assume this is S2_6 patch, and patches for later branches are coming later.

Looks good short of a couple of details:
- What is vsystem used for in unit_veteran_level_string()? Seems like you just get it, but don't use for anything.
- No '//' style comments in C-code
unit_firepower_if_not_one() : "FP:%d " should be translatable. Add TRANS: comment about the fact that the ending space must be there.
- unit_attack_handling() : Add empty line between last of the variable declarations and the code line; before "get_modified_firepower(punit, pdefender, &att_fp, &def_fp);" -line

When porting to later branches, remember that it's possible that both parties survive the fight (there's not necessary winner and loser) when Combat_Rounds are in use.
Another detail in master is that new function headers should have the first comment line ending like "...*//**"

#3 Updated by Marko Lindqvist 3 months ago

Thinking it a bit more, we probably shouldn't be adding winning chance as calculated by omniscient server. There can be factors to it that client/player does not, and should not, know. Seeing the actual chance reveals that information.

#4 Updated by Ilkka Virta 3 months ago

Marko Lindqvist wrote:

Thinking it a bit more, we probably shouldn't be adding winning chance as calculated by omniscient server. There can be factors to it that client/player does not, and should not, know. Seeing the actual chance reveals that information.

I don't think it's the winning chance as such, but the attack and defense strengths. The odds should be just a pure function of the strengths+HP+FP, and it's mainly the strengths that can have hidden modifiers. Maybe FP, too.

Of course it's another question entirely if e.g. the presence of the modifiers should become apparent in combat. After all, it's somewhat hard to miss that the city you're attacking has a Coastal Defense or SAM Battery once it starts firing at you. But right now, the final combat strengths are not sent to the client otherwise, right? (e.g. get_total_defense_power seems only called in unit_versus_unit and unit_win_chance, and they don't send it along.)

If the idea is to just send the info the client already gets in a more persistent format (a very good idea!), then A/D/FP and the combat odds probably can't be shown. Well, I didn't double-check what gets sent; the Qt client shows veteran levels and HP for combat participants, and they seem to be what you'd see on the map for a visible unit.

On the other hand, if the idea is to add more data to be shown, it should perhaps be explicitly stated for documentation at least.

#5 Updated by Ilkka Virta 3 months ago

Charlie Galik wrote:

Here's a test of it in action:

Combat: veteran Greek Warriors [id:166 attack:2 HP:10] has a 25.85% chance against the veteran Sumerian Legion [id:157 defense:4 HP:5].
    fc_snprintf(combat_msg, sizeof(combat_msg),
                /* TRANS: "Combat: veteran 1 German Cannon [id:100
                 * attack:6.1 ... HP:20] has a 55.69% the green Polish
                 * Phalanx [id:99 defense:3.5 ... HP:10]." */
                _("Combat: %s %s %s [id:%d attack:%.1g %sHP:%d] has a " 
                  "%.4g%% chance against the %s %s %s [id:%d defense:%.1g" 
                  " %sHP:%d]."),

Veteran Warriors with attack str 2 sound suspect. You very likely want to use %.1f instead of %.1g here.

Other thoughts:
- Should there be a more descriptive verb here? As in "Greek Warriors attack a German Cannon" or something to that effect. Now the text doesn't really say who attacked who (the readers needs to pick up the implication from which unit has the attack value mentioned inside the brackets)
- I think mostly other messages say "Your Warriors", instead of referring to the receiver's units in the third person
- The cases for tired / non-tired attack duplicate almost the same fc_snprintf, it looks to be just one word that differs. Same in the results with the gained veteran level / didn't gain level cases. Could those be built in pieces like the FP string is now? I don't know if that will make translating harder, if the word order required by some language makes it difficult, but reducing the duplication might help future modifications, and make the function a bit cleaner to read. (It's already ~200 lines before this patch.)

#6 Updated by Marko Lindqvist 3 months ago

Ilkka Virta wrote:

Of course it's another question entirely if e.g. the presence of the modifiers should become apparent in combat. After all, it's somewhat hard to miss that the city you're attacking has a Coastal Defense or SAM Battery once it starts firing at you.

Even if combat should reveal that information, this patch is not the right place to do it.
- If it's a rule that such information is revealed, we should reveal it clearly and not as something one can deduct by applying some math to combat values
- One can't tell which of the effect with equal value has been applied. One should be able to tell if 50% bonus was from Coastal Defense firing at you, or had the opponents just 50% higher morale from some Achievement (in a hypothetical ruleset)
- Speaking of morale boost in a custom ruleset; is it really as obvious as Coastal Defense firing you?

#7 Updated by Louis Moureaux 3 months ago

In real life, most units would be able to report back on the kind of opposition they got (the only exception I can think of being missiles). I'm in no way a military expert, but probing enemy defenses sounds like something a good general should do. Gameplay wise, educating players about why they lost (or won) a fight sounds like a legitimate goal.

In every ruleset I've played, it is already possible to get a sense of the probability by watching the HP bars and counting how many hits are taken by the target. Since a combat involves 10-60 rounds, one can already deduce the true probability with quite some certainty from the combat results alone. Of course, this involves a deeper understanding of probabilities and more dedication than the proposed patch.

#8 Updated by Charlie Galik 3 months ago

Marko Lindqvist wrote:

I assume this is S2_6 patch, and patches for later branches are coming later.

Yes. I can do patches for S3_0 and master will after 2.6 patch is finalized.

Looks good short of a couple of details:
- What is vsystem used for in unit_veteran_level_string()? Seems like you just get it, but don't use for anything.
- No '//' style comments in C-code
unit_firepower_if_not_one() : "FP:%d " should be translatable. Add TRANS: comment about the fact that the ending space must be there.
- unit_attack_handling() : Add empty line between last of the variable declarations and the code line; before "get_modified_firepower(punit, pdefender, &att_fp, &def_fp);" -line

All fixed coming in new patch. Thanks for the detailed review.

Marko Lindqvist wrote:

Thinking it a bit more, we probably shouldn't be adding winning chance as calculated by omniscient server. There can be factors to it that client/player does not, and should not, know. Seeing the actual chance reveals that information.

Yes, I realize this is a change, but a change I think will make the players better (and hence the game better). The client does give you a attack/defense calculation when you middle click. But with different rulesets (like I was testing with default ruleset) a new player may literally never know why they won/lost. In fact, I have created spreadsheet (and Louis made it great) to help calculate this information as it's not straightforward calculation. And even with reading the code and ruleset file we still got a defense calculation wrong which Ilkka pointed out months later. And we can both read rulesets and C code. :)

Marko Lindqvist wrote:

Ilkka Virta wrote:

Of course it's another question entirely if e.g. the presence of the modifiers should become apparent in combat. After all, it's somewhat hard to miss that the city you're attacking has a Coastal Defense or SAM Battery once it starts firing at you.

Even if combat should reveal that information, this patch is not the right place to do it.

Where would is better place to do it?

- If it's a rule that such information is revealed, we should reveal it clearly and not as something one can deduct by applying some math to combat values
- One can't tell which of the effect with equal value has been applied. One should be able to tell if 50% bonus was from Coastal Defense firing at you, or had the opponents just 50% higher morale from some Achievement (in a hypothetical ruleset)

Agreed. So attacking a city will give you some information, but not necessarily all. You might still need to investigate the city. But to your points, this is already is normally done when attacking. You attack with a unit to see what unit defends. If it's a green warrior you know there's no phalanx inside. Or in the last longturn.net game we would fire a missile at a city to see how it did to see if it had SDI in it or not. But my point is that this is just revealing (and teaching the player) what happened.

Ilkka Virta wrote:

Charlie Galik wrote:

Here's a test of it in action:

[...]

[...]

Veteran Warriors with attack str 2 sound suspect. You very likely want to use %.1f instead of %.1g here.

The default ruleset makes the attack 2 for a veteran warrior. I wanted to use %.1g to show the minimum number of characters, so attack of one doesn't read 1.0 instead just shows 1

Other thoughts:
- Should there be a more descriptive verb here? As in "Greek Warriors attack a German Cannon" or something to that effect. Now the text doesn't really say who attacked who (the readers needs to pick up the implication from which unit has the attack value mentioned inside the brackets)

Agreed, fix coming.

- I think mostly other messages say "Your Warriors", instead of referring to the receiver's units in the third person

I purposefully took this out as I have long term (and longturn) hopes that others who have visibility will get the same message. Right now in team games, you see the combat, can have a gui option to center on the combat, but get no message unless it's your unit. Then you message your teammate to send you their messages to see what happened. But I realize that a different thing altogether and out of the scope of this patch.

- The cases for tired / non-tired attack duplicate almost the same fc_snprintf, it looks to be just one word that differs. Same in the results with the gained veteran level / didn't gain level cases. Could those be built in pieces like the FP string is now? I don't know if that will make translating harder, if the word order required by some language makes it difficult, but reducing the duplication might help future modifications, and make the function a bit cleaner to read. (It's already ~200 lines before this patch.)

Yeah I broke it up to make the translation easier.

#9 Updated by Marko Lindqvist 3 months ago

  • Sprint/Milestone changed from 2.6.4 to 3.0.0-beta1

Charlie Galik wrote:

Marko Lindqvist wrote:

Even if combat should reveal that information, this patch is not the right place to do it.

Where would is better place to do it?

New ticket that implements revealing of that information in a clear way. Need to calculate it from combat values is not a way to treat a rule that player should know (be told) that "There is a Coastal Defense in the city". Simplest is probably adding a new message that tells the revealed information.
Such a rule change is not acceptable to S2_6. Retargeting to (at least) S3_0.

#10 Updated by Ilkka Virta 3 months ago

Charlie Galik wrote:

The default ruleset makes the attack 2 for a veteran warrior. I wanted to use %.1g to show the minimum number of characters, so attack of one doesn't read 1.0 instead just shows 1

No, the point is that it doesn't.

S2_6/data/classic/units.ruleset has:

veteran_names = _("green"), _("veteran"), _("hardened"), _("elite")
veteran_power_fact = 100, 150, 175, 200

making a veteran Warriors have attack str 1.5. First veteran level giving +50 % is the same in the usual rulesets.

For %g, my man page says "the precision specifies the number of significant digits", and you've given a precision of one, so it shows
just one significant digit. 1.5 rounded is 2. Also, something like 12 would show as 1e+01. The meaning of the "precision" field is different in %f vs %g.

As for 1.0 vs 1, I would argue it's better to show that one digit after the decimal point in any case, just so the reader doesn't need to wonder if that's an exact or a rounded value. With %.1f, it would exactly match the precision used internally in the default case or POWER_FACTOR = 10. If someone changes the define, they'd get rounding again, of course.

- I think mostly other messages say "Your Warriors", instead of referring to the receiver's units in the third person

I purposefully took this out as I have long term (and longturn) hopes that others who have visibility will get the same message.

That's not such a bad idea either, actually. But I wonder if "your" instead of "Greek" would still be friendlier to the reader to tell those cases apart, though it is more work code-wise.

- The cases for tired / non-tired attack duplicate almost the same fc_snprintf, it looks to be just one word that differs.

Yeah I broke it up to make the translation easier.

Splitting the sending of those messages to a separate function also crossed my mind.

#11 Updated by Ilkka Virta 3 months ago

Marko Lindqvist wrote:

Charlie Galik wrote:

Marko Lindqvist wrote:

Even if combat should reveal that information, this patch is not the right place to do it.

Where would is better place to do it?

New ticket that implements revealing of that information in a clear way. Need to calculate it from combat values is not a way to treat a rule that player should know (be told) that "There is a Coastal Defense in the city". Simplest is probably adding a new message that tells the revealed information.
Such a rule change is not acceptable to S2_6. Retargeting to (at least) S3_0.

Ok, more thoughts. Sorry, this got long.

We have, on one hand, a suggestion to have messages telling the data the client already gets if online; and on the other hand the suggestion to show more of the combat details. The first one doesn't involve any rules changes and only adds clarity, so it might not be a bad change even for a stable branch.

...

The second one would be a change though. One that, I think could be argued both ways. Should a single battle be enough to tell the enemy fought more fiercely than expected, or should it fall to the brass (the player) to figure that out from the losses?

I do agree that showing only the final numbers without telling the factors that affect them is not optimal, but it could still help in figuring out what is probably the most complex part of the game after city turn processing. Sufficiently dedicated players could check their guesses of the enemy capabilities and newer players would also be helped in getting an idea of the numbers involved. In a "shortturn" game one might not have time to double-check the numbers, but in a "longturn" game... it wouldn't seem be outside the realm of what players do. Perhaps it should just be shoved to a ruleset option, punting the responsibility of the change to whoever is running the game? ;)

...

Anyway, there doesn't seem to be a very good way to show the breakdown data without more changes to the client (and the network protocol?).

The list of factors that can affect the strength of a unit is long, so listing them in messages would also quickly become way too verbose. Even if we don't list veteran levels, terrain, rivers and (automatic) fortification (in a city), because the player is just supposed "to know" them, a unit defending a city in e.g. civ2civ3 could gain bonuses from several effects. Should they all be printed? For every attack to the same city?? Should the server do some caching and only tell the details once? Should the client interpret those messages and hide the redundant ones? Should stuff like BadCityDefender be pointed out explicitly, or should they fall to the "you should know that" realm?

If there was no regard for the work needed, I'd suggest changing the network protocol to send all that data in some combat information message, and having the client show it only if the user goes looking. This is what the client already does for city production values: it shows totals, and lets the user hover for the breakdown.

Finding out all effects that contribute to a unit's strength would also depend on the ruleset details. Consider Great Wall in 'classic', it acts like a City Walls in every city. The effects are written such that City Walls only gives a bonus if the player doesn't have the Great Wall. Which means that a combat breakdown would show no defense bonus from City Walls, even though the city you attacked obviously had them. It would show Great Wall though, so you could go back to looking at that and figuring why City Walls didn't show up. On the other hand, if the effects were written the other way around, City Walls could cover the fact that there's also the Great Wall. Should it?

...

Now, currently, even without code changes, city improvements are easy to make visible for someone who wants it: just add the "visible to others" flag that makes them visible on alt-click, the way small wonders and City Walls are. They'll show even before you attack, of course. I think that already goes a long way, actually. Great Wonders are publicly visible, and for small wonders, I think Ignatus has said that the server actually leaks the information of them existing already. If that's true, the client could be modified to list them, so you could figure out Great Wall too before attacking. I'm not sure if there's a ready-made way for achievements to show, and for arbitrary effects, it gets even harder.

#12 Updated by Charlie Galik 3 months ago

Ilkka Virta wrote:

For %g, my man page says "the precision specifies the number of significant digits", and you've given a precision of one, so it shows
just one significant digit. 1.5 rounded is 2. Also, something like 12 would show as 1e+01. The meaning of the "precision" field is different in %f vs %g.

Oops! Thanks for explaining my bug so patiently. :) Fixing.

As for 1.0 vs 1, I would argue it's better to show that one digit after the decimal point in any case, just so the reader doesn't need to wonder if that's an exact or a rounded value. With %.1f, it would exactly match the precision used internally in the default case or POWER_FACTOR = 10. If someone changes the define, they'd get rounding again, of course.

Sounds good, I'll do 1.0. What's the best way to handle POWER_FACTOR? I could do something like this inline:
printf("%.*f", (int)log10f(POWER_FACTOR ), x);
Or even make a function that includes range checking on it :) I don't mind, but I think it's over complicating things. :) Since POWER_FACTOR is used in the print easy to find it when/if it changes as it's pretty rare. So think best to leave it .1f as worst case it is not showing more details, but keeps the message from getting too long as well.

- I think mostly other messages say "Your Warriors", instead of referring to the receiver's units in the third person

I purposefully took this out as I have long term (and longturn) hopes that others who have visibility will get the same message.

That's not such a bad idea either, actually. But I wonder if "your" instead of "Greek" would still be friendlier to the reader to tell those cases apart, though it is more work code-wise.

I agree. But will it make harder on the translation? If not, then for sure I will change it.

- The cases for tired / non-tired attack duplicate almost the same fc_snprintf, it looks to be just one word that differs.

Yeah I broke it up to make the translation easier.

Splitting the sending of those messages to a separate function also crossed my mind.

Yeah sounds good, can refactor to make a function to print tired attack or not, if that's preferred with more complicated print function.

Marko Lindqvist wrote:

We have, on one hand, a suggestion to have messages telling the data the client already gets if online; and on the other hand the suggestion to show more of the combat details. The first one doesn't involve any rules changes and only adds clarity, so it might not be a bad change even for a stable branch.

I agree and want to keep working towards getting something in stable branch. What about for stable branch attackers not being able to see defense and percent chance? So then defenders would get to see all the info, as I'm not aware of any secret attacker modifiers possible in 2.6.X.

Marko Lindqvist wrote:

I do agree that showing only the final numbers without telling the factors that affect them is not optimal, but it could still help in figuring out what is probably the most complex part of the game after city turn processing. Sufficiently dedicated players could check their guesses of the enemy capabilities and newer players would also be helped in getting an idea of the numbers involved. In a "shortturn" game one might not have time to double-check the numbers, but in a "longturn" game... it wouldn't seem be outside the realm of what players do.

Absolutely, us longturn players have way too much time on our hands while waiting for the Turn Change. :)

Marko Lindqvist wrote:

Perhaps it should just be shoved to a ruleset option, punting the responsibility of the change to whoever is running the game? ;)

I too could see this. Do you want (possibly secret) defense values to be revealed when attacking or want the user to guess from the attack outcome? Some games might prefer the stealth.

Marko Lindqvist wrote:

If there was no regard for the work needed, I'd suggest changing the network protocol to send all that data in some combat information message, and having the client show it only if the user goes looking.

Yes, this would be ideal, some "Combat Info Message", then client can easily uncheck them to filter them out if doesn't want to see them.

Marko Lindqvist wrote:

The list of factors that can affect the strength of a unit is long, so listing them in messages would also quickly become way too verbose. Even if we don't list veteran levels, terrain, rivers and (automatic) fortification (in a city), because the player is just supposed "to know" them, a unit defending a city in e.g. civ2civ3 could gain bonuses from several effects. Should they all be printed? For every attack to the same city?? Should the server do some caching and only tell the details once? Should the client interpret those messages and hide the redundant ones? Should stuff like BadCityDefender be pointed out explicitly, or should they fall to the "you should know that" realm?

I think if player is interested (average player isn't) then can go through the help/ruleset to figure out how it got to that answer. But to me the import thing is to give them the answer so they can work backwards to figure it out. I have gotten the client's middle click attack/defense chances to match my local calculation, but took a lot of work. I guess one way would be some sort of combat simulator in the client, that sends the info to the server that reports back. So in game a player could enter in: what is a v canon 2/3 tired attack vs vvv phal in city on hill with river behind walls that is a size 9 that has only 1 HP left? Right now I do that calculation offline in the spreadsheet. But a client feature enhancement is off patch topic. :)

#13 Updated by Charlie Galik 3 months ago

Request For Comments @Marko, @Ilkka, @Louis, @Kevin551 or anyone else :) on the attached patch.

All the above fixes should be included. Example text below.

Combat: Your green Warriors [id:135 attack:1.0 HP:10] has a 4.275% chance attacking the Greek green Warriors [id:165 defense:2.2 HP:10].
Combat result: Your Warriors attack succeeded against the Greek Warriors. Your Warriors lost 9 HP, has 1 HP remaining and achieved the rank of veteran.
Combat: Your green Warriors [id:137 attack:1.0 HP:10] has a 4.275% chance attacking the Greek green Warriors [id:171 defense:2.2 HP:10].
Combat result: Your Warriors attack failed against the Greek Warriors [lost 3 HP, 7 HP remaining and achieved the rank of veteran].
Combat: Greek veteran Warriors [id:166 attack:1.5 HP:10] has a 25.85% chance against your green Legion [id:157 defense:4.5 HP:5].
Combat result: Greek Warriors attack failed against the your Legion [lost 4 HP, 1 HP remaining and achieved the rank of veteran].

Questions still outstanding:
  1. For stable branch will it be acceptable for attacker not to see defense value and percent chance to win? But still let defender see all the info.
  2. Would you like ruleset option to show this info? If hiding it in stable branch, I like this as an option too and can try and code it up.
  3. For future networks protocol with a "Combat Info" message, is it ok to reserve a message now in 3.0 and implement it later?

#14 Updated by Charlie Galik 3 months ago

@Marko, any comments?

Another idea for stable branch, is only this one message change:

Your green Warriors [id:135 attack:1.0 HP:10] attack succeeded against the Greek green Warriors [id:165 HP:10]. Your Warriors lost 9 HP, has 1 HP remaining and achieved the rank of veteran.

If acceptable, I can code up the patch.

#15 Updated by Marko Lindqvist 3 months ago

  • Status changed from New to In Progress

Charlie Galik wrote:

Another idea for stable branch, is only this one message change:
[...]
If acceptable, I can code up the patch.

Yes, I think this is what we should do in the first phase (this ticket). Please make it for all branches, so the development branches will not be left behind S2_6.
Further development for development branches to a new ticket(s).

#16 Updated by Charlie Galik 3 months ago

Sounds good. Here's the a patch for 2.6. If looks good, I can proceed with 3_0 an master.

Here's some example of the updated messages:

Your attacking green Warriors [id:135 A:1.0 lost 9 HP, has 1 remaining] succeeded against the Greek green Warriors [id:165 HP:10].
Your attacking green Warriors [id:137 A:1.0 HP:10] failed against the Greek green Warriors [id:171 lost 3 HP, 7 HP remaining and achieved the rank of veteran]!
Your green Legion [id:157 D:4.5 lost 4 HP, 1 HP remaining] survived the pathetic attack from the Greek veteran Warriors [id:166 A:1.5 HP:10].

#17 Updated by Marko Lindqvist 3 months ago

"and achieved the rank of %s" should be marked for translation.

For right-to-left languages the TRANS: comment for the string where that "and..." -fragment is used should mention it. So that translator knows to adjust placement of the "%s" and contents of the fragment accordingly.

#18 Updated by Charlie Galik 3 months ago

Thanks, Marko. Here's updated 2_6 patch.

I'm not exactly sure best way to do what you're asking. How does that look?

* last %s is either "and ..." or empty string */

#19 Updated by Marko Lindqvist 3 months ago

  • Sprint/Milestone changed from 3.0.0-beta1 to 2.6.4

#20 Updated by Marko Lindqvist 3 months ago

Charlie Galik wrote:

I'm not exactly sure best way to do what you're asking. How does that look?
[...]

I think it will suffice for now, so that we get this forward. Please make the S3_0 and master patches.

#21 Updated by Charlie Galik 3 months ago

Marko Lindqvist wrote:

Please make the S3_0 and master patches.

Ok, will do. I'm looking at S3_0 right now. It doesn't look like there's any message sent if EFT_COMBAT_ROUNDS occurs and neither unit dies. So I don't think I should make a new message saying what happened in the combat rounds (with this issue). Later I do think this should be added, but under a separate issue. Sound good?

Updated patches coming soon.

#24 Updated by Charlie Galik 3 months ago

Should I delete the old ones above?

#25 Updated by Marko Lindqvist 3 months ago

I haven't checked the code context outside the patch, but I'm surprised that S3_0 & master versions do not add any message where HP of both attacker as defender change while both survive the combat (due to COMBAT_ROUNDS limit)

#26 Updated by Charlie Galik 3 months ago

Marko Lindqvist wrote:

I haven't checked the code context outside the patch, but I'm surprised that S3_0 & master versions do not add any message where HP of both attacker as defender change while both survive the combat (due to COMBAT_ROUNDS limit)

Yeah I mentioned it above:
Charlie Galik wrote:

Ok, will do. I'm looking at S3_0 right now. It doesn't look like there's any message sent if EFT_COMBAT_ROUNDS occurs and neither unit dies. So I don't think I should make a new message saying what happened in the combat rounds (with this issue). Later I do think this should be added, but under a separate issue. Sound good?

So I only changed the messages when unit dies. If there's Combat Rounds and neither unit dies, there's no message and it returns:
https://github.com/freeciv/freeciv/blob/S3_0/server/unithand.c#L3914
The current messages are only win/loss attacker/defender. A new message should be added, something like "Combat Info". We can discuss here if you want about adding it, but I think a new message type is outside of the scope of this patch.

#27 Updated by Marko Lindqvist 3 months ago

Tried to build master (--enable-debug so -Werror in use):

../../../src/common/featured_text.c: In function ‘unit_veteran_level_string’:
../../../src/common/featured_text.c:1128:35: error: zero-length gnu_printf format string [-Werror=format-zero-length]
1128 | fc_snprintf(buf, sizeof(buf), ""); | ^~
../../../src/common/featured_text.c: In function ‘unit_tired_attack_string’:
../../../src/common/featured_text.c:1163:7: error: implicit declaration of function ‘is_tired_attack’ [-Werror=implicit-function-declaration]
1163 | if (is_tired_attack(punit->moves_left)) { | ^~~~~~~~~~~~~
../../../src/common/featured_text.c:1163:7: error: nested extern declaration of ‘is_tired_attack’ [-Werror=nested-externs]
../../../src/common/featured_text.c:1168:35: error: zero-length gnu_printf format string [-Werror=format-zero-length]
1168 | fc_snprintf(buf, sizeof(buf), ""); | ^~
../../../src/common/featured_text.c: In function ‘unit_firepower_if_not_one’:
../../../src/common/featured_text.c:1185:35: error: zero-length gnu_printf format string [-Werror=format-zero-length]
1185 | fc_snprintf(buf, sizeof(buf), ""); | ^~
cc1: all warnings being treated as errors

#28 Updated by Charlie Galik 3 months ago

Will compile with --enable-debug and fix. Sorry about that.

#30 Updated by Marko Lindqvist 2 months ago

  • Status changed from In Progress to Resolved
  • Assignee changed from Charlie Galik to Marko Lindqvist

#31 Updated by Marko Lindqvist 2 months ago

  • Status changed from Resolved to Closed

#32 Updated by Marko Lindqvist about 1 month ago

#33 Updated by Lexxie L 29 days ago

just an innocent question, but why give the id of the unit?

#34 Updated by Alexandro Ignatiev 28 days ago

Likely, to track which unit has fought which one. Most homebrew combat informers print this info. If you fight a phalanx in a city with hp falling 15 to 10 and next attack is met by 10hp phalanx, you are interested are they the same unit or just some other phal happened to have 10hp in the same place.

#35 Updated by Ilkka Virta 28 days ago

Lexxie L wrote:

why give the id of the unit?

As far as I understand, the ids are also part of what the server sends to the client anyway, if the client is connected at the time and the player can see the battle (packet_unit_combat_info contains unit ids). The idea here is to have the relevant info available even if the player is offline at the time.

Also available in: Atom PDF