triplea-game / triplea

TripleA is a turn based strategy game and board game engine, similar to Axis & Allies or Risk.
https://triplea-game.org/
GNU General Public License v3.0
1.3k stars 387 forks source link

I think this may be a logic problem #12593

Open WCSumpton opened 2 months ago

WCSumpton commented 2 months ago

https://github.com/triplea-game/triplea/blob/7aeff7e35a7c53a3feee535cbc5167c10b42dc60/game-app/game-core/src/main/java/games/strategy/triplea/delegate/power/calculator/AvailableSupports.java#L167

The comment above getNextAvailableSupporter states that the same unit (not unitType) maybe returned multiple times.

If I'm not mistaken giveSupportToUnit is a unit-to-unit comparison and not unitType. A single unit may support another unit on a one-to-one basis. An Artillery may support An Infantry. Even if number is set to 2. Then that Artillery may support another Infantry only if that second Infantry is presence. If bonusType's count is set to 2, the lone Artillery may still only support one Infantry with the bonus amount and if there are two, both would receive the bonus amount. It takes the presence of a second Artillery before bonusType's count has an effect.

Thoughts...

Cheers...

asvitkine commented 2 months ago

An Artillery may support An Infantry. Even if number is set to 2. Then that Artillery may support another Infantry only if that second Infantry is presence.

This part of your message says that an artillery can support two infantry. So I think that's what the above is doing - i.e. for each supporting unit, keep track how many units it can support.

Cernelius commented 2 months ago

To clarify with a simple example, we can have these rules.

Under these rules, we can have the situation of 1 artillery and 2 infantries offensively attacking.

We can foresee three different reasonable outcomes, depending on the rules.

  1. Both infantries attack at 2 (+1 each).
  2. One infantry attacks at 3 (+2) and the other one at 2 (+1).
  3. One infantry attacks at 4 (+3) and the other one at 1 (+0).

Notice that in the first case one of the three support abilities of the artillery is non-used, whereas in the second and third cases them all are used.

@WCSumpton Are you saying that (in your opinion) the program may be expected to behave as per point 3 according to the comment but actually behaves as per point 1, and this actual behaviour is correct? Meaning that the behaviour is correct and the comment is either wrong or unclear, so the comment should be improved to better define the behaviour (and the behaviour should be kept just as it is)?


Anyway, the best reference here would be @ron-murhammer if he's available for providing any insight. Did he write the comment which you are referencing? If not, who did?

WCSumpton commented 2 months ago
  • The infantry attacks at 1 and can receive support from up to 3 artilleries (attacking at 2 if receiving support from 1, attacking at 3 if receiving support from 2 and attacking at 4 if receiving support from 3).

I think I get what you are saying here.

Under these rules, we can have the situation of 1 artillery and 2 infantries offensively attacking.

Ok, should be the first example, both infantries attack at 2 (+1 each). Because only 1 artillery is attacking. As of now it is one infantry attacks at 4 (+3) and the other one at 1 (+0).

Thanks for your help @Cernelius

Cheers...

Cernelius commented 2 months ago

I understood that the program's behaviour would actually be point 1 from what you said about your testing for @beelee1's "wolfpacks".

WCSumpton commented 2 months ago

@Cernelius

Yes, I know that. But after further testing, I have determined that the program functions are in line with number 3. I have spent the past couple of days testing and checking this. That is why I posted this. I feel that this operation is in error according to the description in PoS2, and should be changed to reflect option 1.

But this is my own stated opinion. As it set the self-buffing/debuffing I described for @beelee1 is impossible. I would just like to understand why it was programmed like this but described in a different way.

It would also be nice if other member voiced their opinions.

Thanks again

Cheers...

beelee1 commented 2 months ago

@WCSumpton

As it set the self-buffing/debuffing I described for @beelee1 is impossible.

oh oh that doesn't sound good :) Haven't noticed any incorrect behavior with your "variable" suggestion. Haven't tested extensively yet.

Anyway, Idk if this is pertinent or not, but I have a Elite Infantry that gets 2 rolls in combat. Artillery gives both rolls a +1. Regular Infantry get 1 roll and also receive the +1 bonus from the Artillery. The Artillery cannot support both at the same time though.

What happens, is sometimes the Artillery will boost both Elite rolls and none for the Regular Inf and sometimes 1 roll as well as a Regular Inf roll. The result is the same number of rolls boost wise.

This may or not matter as well but I have a unit that gives a combat roll boost to multiple units that have different combat values. The unit that receives the boost seems to be randomly selected or maybe it was in order of unit list in the xml. I should revisit that as I could probably make it work then.

At any rate, I ended up making a different boost unit for each unit that had a different combat value and use edit to have the desired unit present.

Disregard if this is all off base. Not that you would need me to tell you that lol

Peace

Edit I should have reread sooner, it seems this is a different issue than what I thought :)

Cernelius commented 2 months ago

@Cernelius

Yes, I know that. But after further testing, I have determined that the program functions are in line with number 3. I have spent the past couple of days testing and checking this. That is why I posted this. I feel that this operation is in error according to the description in PoS2, and should be changed to reflect option 1.

But this is my own stated opinion. As it set the self-buffing/debuffing I described for @beelee1 is impossible. I would just like to understand why it was programmed like this but described in a different way.

That impossibility is what I initially assumed (based on what I remembered from years ago), but I understood that you tested it successfully.

Clearly, I need to do some testing.

Now, from a rule-book perspective, if we forget that this is a computer game and instead assume that we are dealing with a traditional board-game (where everything is handled manually), the simplest example of what I suppose we would be asking to the person who wrote that rule-book is "if I have an infantry which can be supported by up to two artilleries and an artillery which can support up to two infantries, can the infantry be supported twice by a single artillery (or is it needed two different artilleries supporting the same infantry in order for the infantry to be supported twice)?".

Unfortunately, TripleA is not a single board-game with an active author answering questions about how his/her game is supposed to work, so, going back to ourselves, I think that the best solution would be answering "no, it cannot" (which would be the behaviour at point 1, whereas "yes, it can" would be either the behaviour at point 3 or the behaviour at point 2, depending respectively on whether we prefer to stack or not to stack supports upon the same unit when we have the option either to do so or to support an other unit).

An even better solution would be adding an option to let the game-maker (meaning the person writing the XML either with a text editor or with some map-making utility) answer such a question.

Summarizing,

I have determined that the program functions are in line with number 3. I have spent the past couple of days testing and checking this. That is why I posted this. I feel that this operation is in error according to the description in PoS2, and should be changed to reflect option 1.

I agree with your proposal. Are you actually able to do it (beside needing an owner to merge it)?

I'll also underline that I believe that such a change would not remove the possibility to have the point 3 behaviour, as you could still do that by exchanging an attachment with number equal to 2 with two otherwise identical attachments with number equal to 1 (two attachments supporting 1 unit each instead of one attachment supporting 2 units).

However, I cannot exclude that there may be one or more games which may have their behaviours unintentionally changed by this change, but I'm against this being anything which should forestall anything (because I'm almost sure that no main or popular game should be affected in any way, and because it is easy to fix any game (so it will be fixed if it is maintained), and because it does not really matter if nobody cares about it).

All I've said at this post is based on the assumption that what @WCSumpton says is (now) true as I've not tested it yet (but do intend to).

WCSumpton commented 1 month ago

@Cernelius, @asvitkine

These are the corrections that I think might help with solving this problem.

Most of the above I cannot do, I don't have the necessary skill with java. OOP's still give me problems, so I post this plan as a review.

As an aside note, I think a Method giveSupportToUnit would be a nice place to have a combat round check (maxRounds discussion maxRounds discussion here https://github.com/triplea-game/triplea/issues/12488#issuecomment-2105939749 ). A check just before the check for unit is part of the selected rule.

Thoughts and comments welcome!

Cheers...

Cernelius commented 1 month ago

I'd let them have different counts. I'm not seeing why not. The only thing is that it should be decided how the engine should behave when you have more supports than supportable and not all of them have the same count. It should be akin to how the program currently behaves when you have different supports of the same type and not enough supportable units for them all.

That way, for example, you could have light artillery and heavy artillery (or mangonel and trebuchet) with the same type but count 1 and 2 respectively. This would represent the more superficial effect of light artillery (in that a same unit can be supported either by 1 light artillery or by 1 light and 1 heavy artillery or by 2 heavy artilleries but never by 2 light artilleries).

By the way, based on what I remember, the way the same-count thing was enforced (and I would expect still is) was not by forbidding having different counts but by forcing them to be the same. For example, if you had two same types with count 2 and 3, they would both be set at either 2 or 3. I would agree that making the game fail would be better if we really want to have the same count.

I always thought that this thing of the count being specified per attachment but forced to be the same amongst the same type was just wrong. If it is specified per attachment, it should be per attachment (not per type)!

WCSumpton commented 1 month ago

@asvitkine, @Cernelius, @beelee1

I have completed my changes to AvailableSupports, this includes a method, getAt, in CollectionUtils and modification to AvailableSupportsTest. Test twoSupportersInAStackWithSupportNumberOfTwoGiveSupportToTwoUnits was modified to show that supportUnit gave support to both unit and unit2, while supportUnit2 gave stacking support to both unit and unit2.

Should the 3 files be merged for review?

Cheers...