Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some Attacking Units Not Counted When Rolling Dice #1404

Closed
dj2lp opened this issue Dec 19, 2016 · 13 comments
Closed

Some Attacking Units Not Counted When Rolling Dice #1404

dj2lp opened this issue Dec 19, 2016 · 13 comments
Labels
Problem A problem, bug, defect - something to fix

Comments

@dj2lp
Copy link

dj2lp commented Dec 19, 2016

playing revised wih rel 1.9.....0.3277, when attcking a fleet in sz 34, with 6 ftrs and 1 bmb, it only accounted for 5 ftrs and 1 bmb.

Per Ron: Player attacks with 6 fighters and 1 bomber but the first round of combat only rolls for 5 fighters and 1 bomber. Its unclear why that occurs but my best guess is 2 of the fighters have the same UID.

@ron-murhammer
Copy link
Member

@dj2lp Can you provide a save game showing the issue?

@dj2lp
Copy link
Author

dj2lp commented Dec 20, 2016 via email

@ron-murhammer
Copy link
Member

@dj2lp I don't think the attachment was included.

@DanVanAtta
Copy link
Member

@j2lp please double check the attachment, this may be hard to reproduce, the save game if you still have it would be very useful.

@dj2lp
Copy link
Author

dj2lp commented Dec 27, 2016

Hi all

I am unable to load a tsvg here on github (it says they do not support that type). I will send again the attchment by gmail

btw, thanks for your work - hours of joy for plenty of people

@dj2lp
Copy link
Author

dj2lp commented Dec 27, 2016 via email

@DanVanAtta
Copy link
Member

DanVanAtta commented Dec 27, 2016

The tsvg file needs to be zipped first, then github will allow it to be attached
(We have a bug report page in the works, we'll get that detail on that page. With luck the new bug report page will be up in the coming month)

@dj2lp
Copy link
Author

dj2lp commented Dec 29, 2016

ok, I try again. If it works, here it is

kk.3(axis).tsvg.gz

@dj2lp
Copy link
Author

dj2lp commented Dec 29, 2016

seems it worked :)

@ron-murhammer
Copy link
Member

@dj2lp - You are correct that for some reason it only rolls for 5 fighters and 1 bomber instead of 6 fighters and 1 bomber. My guess is somehow 2 fighters have the same UID so it filters one of them out but not sure. Did you happen to use edit mode at all during that game?

@ron-murhammer ron-murhammer added the Problem A problem, bug, defect - something to fix label Jan 10, 2017
@ron-murhammer ron-murhammer changed the title missing units in attack Some Attacking Units Not Counted When Rolling Dice Jan 10, 2017
@Cernelius
Copy link
Contributor

May this be the same issue as this, except that is defence:

#1258

@NikitasKotsolakos
Copy link

NikitasKotsolakos commented Mar 26, 2017

Hello,

I am new in tripleA's community, and recently decided to try to contribute to this game.
I chose this bug as a start, and I have tracked it down, but due to my limited knowledge on game mechanics, I need some help on determining the best solution.

So, the problem in this situation is not some same duplicated ID, but the fact that one of the fighters has the isTransportedBy attribute set. In Brief:

  1. British figher is stationed on an American Carrier. American carrier moves for an attack. This causes the British Fighter to move, but also have the attribute isTransportedBy set (to the american carrier). (I am assuming this is ok and working as intended).
  2. This attribute was not unset when the fighter moved on its own and landed to Caucausus. (This seems wrong, but I am not sure how exatly things are generally handled)
  3. Because of this, when the fighter moved again to fight, a seemingly unrelated check (
    // remove any allied air units that are stuck on damaged carriers (veqryn)
    CompositeMatchAnd<>(Matches.unitIsBeingTransported(),
    Matches.UnitIsAir, Matches.UnitCanLandOnCarrier)
    ) causes it do be removed from the battle, as the british fighter satisfies these 3 clauses (maybe this check should be more specific somehow, but it is not the root of the problem).

So, from my understanding, the correct way to fix it is to make sure that when the fighter moves on its own, it no longer has the transportedBy attribute set. Is this correct? Or this could also be problematic for other situations that I cannot think about as I don't have much experience with the game and the codebase?

If the above is correct, I will start looking for a way to fix it.

P.S. The whole history of the bug as it appears in this specific case (from the kk.3(axis).tsvg.gz savegame dj2lp provided):

  1. Beginning of round 14: British players has 3 Fighters in Sea Zone 12. He has one carrier in that sea zone, while American player has another. So, 2 of the British fighters are stationed in the British carrier, and one is stationed in the american carrier.

  2. Round 14 American Combat Move 1: American player moves his ships, including the carrier to from Sea Zone 12 to Sea Zone 13. This causes the British fighter to also move. At this point, the isTransportedBy attribute is set for the British fighter:
    MustFightBattle.java 212-221:
    // Dependencies count both land and air units. Land units could be allied or owned, while air is just allied // since owned already // launched at beginning of turn fighters.retainAll(Match.getMatches(fighters, Matches.UnitIsAir)); for (final Unit fighter : fighters) { // Set transportedBy for fighter change.add(ChangeFactory.unitPropertyChange(fighter, carrier, TripleAUnit.TRANSPORTED_BY)); } // remove transported fighters from battle display m_attackingUnits.removeAll(fighters);

  3. Round 14 American Combat Sea Zone 13: Americans fight and retreat back to Sea Zone 12.
    Note: The British fighter does not participate in the fight, due to this check:
    MustFightBattle.java
    185 // Filter out allied units if WW2V2 186 final Match<Unit> ownedBy = Matches.unitIsOwnedBy(m_attacker); 187 final Collection<Unit> attackingUnits = isWW2V2() ? Match.getMatches(units, ownedBy) : units;

  4. Round 15 British Non Combat Move 2: British players moves his 3 fighters (including the above one) from Sea Zone 12 to Caucasus. However, the isTransportedBy attribute still stays for the 1 British Fighter.

  5. Round 16 British Combat Move: Fighters moved from Caucasus to 34 Sea Zone.

  6. Round 16 British Combat Battle in 34 Sea Zone:
    The above mentioned fighter that still has the attribute isTransportedBy set to the american carrier, is not taken into account in the battle due to this check:
    MustFightBattle.java
    2338 // remove any allied air units that are stuck on damaged carriers (veqryn) 2339 unitList.removeAll(Match.getMatches(unitList, new CompositeMatchAnd<> (Matches.unitIsBeingTransported(), Matches.UnitIsAir, Matches.UnitCanLandOnCarrier)));

@ron-murhammer
Copy link
Member

@NikitasKotsolakos Your analysis is correct and was very helpful (saved me a bunch of time). I looked into the cause for why the transportedBy attribute wasn't cleared for the UK fighter and submitted a fix in #1777. Thanks for your help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Problem A problem, bug, defect - something to fix
Projects
None yet
Development

No branches or pull requests

5 participants