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.

Sign up for GitHub

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

Jump to bottom

Fix issue 12826 (UnitSeparator#categorize:213 - java.util.ConcurrentModificationException) #12828

Merged
merged 2 commits into from
Aug 9, 2024
Merged

Fix issue 12826 (UnitSeparator#categorize:213 - java.util.ConcurrentModificationException) #12828

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cd6648c
Fix issue 12826 (UnitSeparator#categorize:213 - java.util.ConcurrentM…
frigoref Aug 9, 2024
47db831
PlayerUnitsPanel ToDo done via redraw
frigoref Aug 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions ...-app/game-core/src/main/java/games/strategy/triplea/odds/calculator/PlayerUnitsPanel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import lombok.Getter;
import org.triplea.java.collections.CollectionUtils;
import org.triplea.java.collections.IntegerMap;
import org.triplea.swing.SwingComponents;

/** Panel showing full list of units for a player in a given battle simulation. */
public class PlayerUnitsPanel extends JPanel {
Expand Down Expand Up @@ -102,11 +103,7 @@ public void init(
}
}

// TODO: probably do not need to do this much revalidation.
invalidate();
validate();
revalidate();
getParent().invalidate();
SwingComponents.redraw(this);
}

/**
Expand Down
200 changes: 107 additions & 93 deletions game-app/game-headed/src/main/java/games/strategy/triplea/ui/PlacePanel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.function.Predicate;
Expand All @@ -34,21 +35,71 @@
import javax.swing.JPanel;
import javax.swing.JScrollPane;
import javax.swing.SwingUtilities;
import org.jetbrains.annotations.NotNull;
import org.triplea.java.collections.CollectionUtils;
import org.triplea.swing.SwingComponents;

class PlacePanel extends AbstractMovePanel implements GameDataChangeListener {
private static final long serialVersionUID = -4411301492537704785L;
private final JLabel leftToPlaceLabel = createIndentedLabel();
private PlaceData placeData;
private transient PlaceData placeData;

private final SimpleUnitPanel unitsToPlacePanel;

private GamePlayer lastPlayer;
private boolean postProductionStep;

private final MapSelectionListener placeMapSelectionListener =
private final transient MapSelectionListener placeMapSelectionListener =
new DefaultMapSelectionListener() {

private PlaceableUnits getUnitsToPlace(final Territory territory) {
try (GameData.Unlocker ignored = getData().acquireReadLock()) {
// not our territory
if (!territory.isWater() && !territory.isOwnedBy(getCurrentPlayer())) {
if (GameStepPropertiesHelper.isBid(getData())) {
final PlayerAttachment pa = PlayerAttachment.get(territory.getOwner());
if ((pa == null || !pa.getGiveUnitControl().contains(getCurrentPlayer()))
&& !territory.anyUnitsMatch(Matches.unitIsOwnedBy(getCurrentPlayer()))) {
return new PlaceableUnits();
}
} else {
return new PlaceableUnits();
}
}
// get the units that can be placed on this territory.
Collection<Unit> units = getCurrentPlayer().getUnits();
if (territory.isWater()) {
GameProperties properties = getData().getProperties();
if (!(Properties.getProduceFightersOnCarriers(properties)
|| Properties.getProduceNewFightersOnOldCarriers(properties)
|| Properties.getLhtrCarrierProductionRules(properties)
|| GameStepPropertiesHelper.isBid(getData()))) {
units = CollectionUtils.getMatches(units, Matches.unitIsSea());
} else {
final Predicate<Unit> unitIsSeaOrCanLandOnCarrier =
Matches.unitIsSea().or(Matches.unitCanLandOnCarrier());
units = CollectionUtils.getMatches(units, unitIsSeaOrCanLandOnCarrier);
}
} else {
units = CollectionUtils.getMatches(units, Matches.unitIsNotSea());
}
if (units.isEmpty()) {
return new PlaceableUnits();
}
final IAbstractPlaceDelegate placeDel =
(IAbstractPlaceDelegate) getPlayerBridge().getRemoteDelegate();
final PlaceableUnits production = placeDel.getPlaceableUnits(units, territory);
if (production.isError()) {
JOptionPane.showMessageDialog(
getTopLevelAncestor(),
production.getErrorMessage() + "\n\n",
"Cannot produce units",
JOptionPane.INFORMATION_MESSAGE);
}
return production;
}
}

@Override
public void territorySelected(final Territory territory, final MouseDetails e) {
if (!isActive() || (e.getButton() != MouseEvent.BUTTON1)) {
Expand All @@ -66,21 +117,7 @@ public void territorySelected(final Territory territory, final MouseDetails e) {
if (maxUnits >= 0) {
chooser.setMaxAndShowMaxButton(maxUnits);
}
final Dimension screenResolution = Toolkit.getDefaultToolkit().getScreenSize();
final int availHeight = screenResolution.height - 120;
final int availWidth = screenResolution.width - 40;
final JScrollPane scroll = new JScrollPane(chooser);
scroll.setBorder(BorderFactory.createEmptyBorder());
scroll.setPreferredSize(
new Dimension(
(scroll.getPreferredSize().width > availWidth
? availWidth
: (scroll.getPreferredSize().width
+ (scroll.getPreferredSize().height > availHeight ? 20 : 0))),
(scroll.getPreferredSize().height > availHeight
? availHeight
: (scroll.getPreferredSize().height
+ (scroll.getPreferredSize().width > availWidth ? 26 : 0)))));
final JScrollPane scroll = getScrollPaneFromChooser(chooser);
final int option =
JOptionPane.showOptionDialog(
getTopLevelAncestor(),
Expand All @@ -92,15 +129,43 @@ public void territorySelected(final Territory territory, final MouseDetails e) {
null,
null);
if (option == JOptionPane.OK_OPTION) {
final Collection<Unit> choosen = chooser.getSelected();
placeData = new PlaceData(choosen, territory);
final Collection<Unit> selectedUnits = chooser.getSelected();
placeData = new PlaceData(selectedUnits, territory);
updateUnits();
if (choosen.containsAll(units)) {
if (selectedUnits.containsAll(units)) {
leftToPlaceLabel.setText("");
}
release();
}
}

@NotNull
private JScrollPane getScrollPaneFromChooser(final UnitChooser chooser) {
final Dimension screenResolution = Toolkit.getDefaultToolkit().getScreenSize();
final int availHeight = screenResolution.height - 120;
final int availWidth = screenResolution.width - 40;
final JScrollPane scroll = new JScrollPane(chooser);
scroll.setBorder(BorderFactory.createEmptyBorder());
scroll.setPreferredSize(
new Dimension(
(scroll.getPreferredSize().width > availWidth
? availWidth
: getPreferredWith(scroll, availHeight)),
(scroll.getPreferredSize().height > availHeight
? availHeight
: getPreferredHeight(scroll, availWidth))));
return scroll;
}

private int getPreferredHeight(final JScrollPane scroll, final int availWidth) {
return scroll.getPreferredSize().height
+ (scroll.getPreferredSize().width > availWidth ? 26 : 0);
}

private int getPreferredWith(final JScrollPane scroll, final int availHeight) {
return scroll.getPreferredSize().width
+ (scroll.getPreferredSize().height > availHeight ? 20 : 0);
}
};

PlacePanel(final GameData data, final MapPanel map, final TripleAFrame frame) {
Expand Down Expand Up @@ -147,42 +212,39 @@ private void updateStep() {
}

if (showUnitsToPlace) {
// Small hack: copy the unit list before passing it to a new thread.
// This is to prevent ConcurrentModification. If the 'unitsToPlace' list is modified
// later in this thread, before "SwingUtilities.invokeLater" can execute and complete,
// then we will get a ConcurrentModification exception.
// Ideally we would not modify the 'unitsToPlace' collection again except when
// the swing thread signals that the user has taken action.. Short of that, we create a copy
// here.
final Collection<Unit> unitsToPlaceCopy = new ArrayList<>(unitsToPlace);
SwingUtilities.invokeLater(
() -> {
unitsToPlacePanel.setUnits(unitsToPlaceCopy);
SwingComponents.redraw(unitsToPlacePanel);
});
updateUnitsInUnitsToPlacePanel(unitsToPlace);
} else {
SwingUtilities.invokeLater(unitsToPlacePanel::removeAll);
}
}

private void updateUnitsInUnitsToPlacePanel(final Collection<Unit> newUnitsToPlace) {
// Small hack: copy the unit list before passing it to a new thread.
// This is to prevent ConcurrentModification. If the 'unitsToPlace' list is modified
// later in this thread, before "SwingUtilities.invokeLater" can execute and complete,
// then we will get a ConcurrentModification exception.
// Ideally we would not modify the 'unitsToPlace' collection again except when
// the swing thread signals that the user has taken action.
// Short of that, we create a copy here.
final Collection<Unit> unitsToPlaceCopy =
Collections.unmodifiableCollection(new ArrayList<>(newUnitsToPlace));
SwingUtilities.invokeLater(
() -> {
unitsToPlacePanel.setUnits(unitsToPlaceCopy);
SwingComponents.redraw(unitsToPlacePanel);
});
}

@Override
public void gameDataChanged(final Change change) {
final Collection<Unit> unitsToPlace;
final GameData data = getData();
try (GameData.Unlocker ignored = data.acquireReadLock()) {
final GamePlayer player = data.getSequence().getStep().getPlayerId();
if (player == null) {
return;
if (player != null) {
final Collection<Unit> unitsToPlace = player.getUnits();
updateUnitsInUnitsToPlacePanel(unitsToPlace);
}
unitsToPlace = player.getUnits();
}

SwingUtilities.invokeLater(
() -> {
unitsToPlacePanel.setUnits(unitsToPlace);
unitsToPlacePanel.revalidate();
unitsToPlacePanel.repaint();
});
}

@Override
Expand Down Expand Up @@ -210,54 +272,6 @@ PlaceData waitForPlace(final boolean bid, final PlayerBridge playerBridge) {
return placeData;
}

private PlaceableUnits getUnitsToPlace(final Territory territory) {
try (GameData.Unlocker ignored = getData().acquireReadLock()) {
// not our territory
if (!territory.isWater() && !territory.isOwnedBy(getCurrentPlayer())) {
if (GameStepPropertiesHelper.isBid(getData())) {
final PlayerAttachment pa = PlayerAttachment.get(territory.getOwner());
if ((pa == null || !pa.getGiveUnitControl().contains(getCurrentPlayer()))
&& !territory.anyUnitsMatch(Matches.unitIsOwnedBy(getCurrentPlayer()))) {
return new PlaceableUnits();
}
} else {
return new PlaceableUnits();
}
}
// get the units that can be placed on this territory.
Collection<Unit> units = getCurrentPlayer().getUnits();
if (territory.isWater()) {
GameProperties properties = getData().getProperties();
if (!(Properties.getProduceFightersOnCarriers(properties)
|| Properties.getProduceNewFightersOnOldCarriers(properties)
|| Properties.getLhtrCarrierProductionRules(properties)
|| GameStepPropertiesHelper.isBid(getData()))) {
units = CollectionUtils.getMatches(units, Matches.unitIsSea());
} else {
final Predicate<Unit> unitIsSeaOrCanLandOnCarrier =
Matches.unitIsSea().or(Matches.unitCanLandOnCarrier());
units = CollectionUtils.getMatches(units, unitIsSeaOrCanLandOnCarrier);
}
} else {
units = CollectionUtils.getMatches(units, Matches.unitIsNotSea());
}
if (units.isEmpty()) {
return new PlaceableUnits();
}
final IAbstractPlaceDelegate placeDel =
(IAbstractPlaceDelegate) getPlayerBridge().getRemoteDelegate();
final PlaceableUnits production = placeDel.getPlaceableUnits(units, territory);
if (production.isError()) {
JOptionPane.showMessageDialog(
getTopLevelAncestor(),
production.getErrorMessage() + "\n\n",
"Cannot produce units",
JOptionPane.INFORMATION_MESSAGE);
}
return production;
}
}

@Override
public String toString() {
return "PlacePanel";
Expand Down Expand Up @@ -315,6 +329,6 @@ protected final List<Component> getAdditionalButtons() {
}

private void updateUnits() {
unitsToPlacePanel.setUnits(getCurrentPlayer().getUnits());
updateUnitsInUnitsToPlacePanel(getCurrentPlayer().getUnits());
}
}