Improve carrier landing validation for air units in the placement phase#14505
Open
VictoryFirst1 wants to merge 3 commits into
Open
Improve carrier landing validation for air units in the placement phase#14505VictoryFirst1 wants to merge 3 commits into
VictoryFirst1 wants to merge 3 commits into
Conversation
VictoryFirst1
force-pushed
the
prevent-placement-on-friendly-carriers
branch
from
044cf6d to
8f244bc
Compare
Improves carrier landing validation for air units in the placement phase Prevents a player from placing air units on a friendly carrier When there is a friendly carrier in a sea zone, normally the engine correctly prevents you from placing your aircraft directly in this sea zone, EXCEPT when there is a carrier controlled by you in that sea zone (either from a previous turn or placed there this turn). In that case, the game does allow you to place aircraft on the friendly carrier, in addition to the carrier you own. This fixes that issue, as well as the issue where fighters you OWN already in the territory you want to place fighters on carriers weren't taken into account. What previously happened was that the game allowed you to place let's say two fighters in a sea zone containing an aircraft carrier and two fighters. The game only recognized the carrier and therefore allowed the placement of two more fighters, but ignored the two fighters already present. Then, if you clicked "done", the engine would warn you of fighters not being able to land. Now, the engine will immediately tell you that you cannot place those fighters in that territory because there are not enough carriers. TODO: Currently, there is one situation where the game still allows you to place fighters on friendly carriers. That is when the sea zone contains: 1) A carrier owned by you 2) A friendly carrier 3) Aircraft from the friendly power This is because at the time of writing, TripleA does not know which fighters are on which carriers In the case the friendly aircraft are on the friendly carrier, you CAN place air units on your empty carrier. If the friendly aircraft are on your OWN carrier, then you CANNOT place air units in the sea zone because you cannot place air units on the friendly carrier Currently in this last situation, the game does allow you to place aircraft in the sea zone.
VictoryFirst1
force-pushed
the
prevent-placement-on-friendly-carriers
branch
from
8f244bc to
1f7cd14
Compare
frigoref
reviewed
Jun 1, 2026
frigoref
reviewed
Jun 1, 2026
Replaced the for loop to find the units owned by the player with: to.getMatches(Matches.unitIsOwnedBy(player)) Removed the one line comments that explain the code Removed a comment block above my code that appeared to be redundant. Modified the TODO comment to be slightly be more concise.
VictoryFirst1
changed the title
DanVanAtta
reviewed
Jun 6, 2026
…te/AbstractPlaceDelegate.java Good suggestion, it's better if these comments are removed entirely Co-authored-by: Dan Van Atta <dhvatta@gmail.com>
Member
Author
|
@DanVanAtta Updated the PR again, it's ready for a re-review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When there is a friendly carrier in a sea zone, normally the engine correctly prevents you from placing your aircraft directly in this sea zone, EXCEPT when there is a carrier controlled by you in that sea zone (either from a previous turn or placed there this turn). In that case, the game does allow you to place aircraft on the friendly carrier, in addition to on the carrier you own.
This PR fixes that issue, as well as the issue where fighters you OWN already in the sea zone that you want to place fighters on carriers weren't taken into account. What previously happened was that the game allowed you to place let's say two fighters in a sea zone containing an aircraft carrier and two fighters. The game only recognized the carrier and therefore allowed the placement of two more fighters, but ignored the two fighters already present. Then, if you clicked "done", the engine would warn you of fighters not being able to land. Now, the engine will immediately tell you that you cannot place those fighters in that territory because there are not enough carriers. Based on the removed TODO comment that I happened to notice.
Attempts to fix: #6238
Notes to Reviewer
TODO: Currently there is still one scenario where the game incorrectly allows you to place fighters on friendly carriers. That is when a sea zone contains:
This is because at the time of writing, TripleA does not know which fighters are on which carriers.
If the friendly air units are on the friendly carrier, you should be able to place air units on your empty carrier.
If the friendly air units are on your OWN carrier, then you should NOT be able to place air units in the sea zone because you are not allowed to place air units on the friendly carrier.
Currently in this last situation, the game incorrectly allows you to place aircraft in the sea zone.
Because aircraft assigned to carriers is not implemented yet, there is no way to distinguish if the first situation is occurring or the second situation. Therefore, I think it's best to leave that problem as it is until this has been implemented and simply allow it for both instances. It is better to allow it in an incorrect case than disallow it in a correct case.