Fix temporary bonus target selection to show all matching equipment

[Copilot]

Problem

When selecting a temporary bonus target (e.g., "Magic Weapon" spell) in the Temporary Bonuses tab, only one weapon was displayed in the chooser dialog instead of all applicable weapons. This issue was reported in #[issue_number] where users with multiple weapons (e.g., both a sling and longbow) could only see one weapon (the sling) as a valid target for spells like Gravity Bow.

Root Cause

The bug was in the TempBonusHelper.getListOfApplicableEquipment() method. The code used a labeled break statement (break FINDEQ;) that incorrectly exited the outer equipment iteration loop instead of just the inner bonus condition checking loop:

FINDEQ: for (Equipment aEq : charDisplay.getEquipmentSet())
{
    for (EquipBonus eb : originObj.getSafeListFor(ListKey.BONUS_EQUIP))
    {
        if (passesConditions(aEq, eb.conditions))
        {
            possibleEquipment.add(aEq);
            break FINDEQ;  // BUG: Exits outer loop entirely!
        }
    }
}

This meant that as soon as the first matching equipment was found (typically the first alphabetically), the method would stop checking the remaining equipment and return a list with only one item.

Solution

Removed the FINDEQ: label and changed break FINDEQ; to break; so that only the inner loop is exited:

for (Equipment aEq : charDisplay.getEquipmentSet())
{
    for (EquipBonus eb : originObj.getSafeListFor(ListKey.BONUS_EQUIP))
    {
        if (passesConditions(aEq, eb.conditions))
        {
            possibleEquipment.add(aEq);
            break;  // Only exits inner loop, continues to next equipment
        }
    }
}

This allows the method to continue iterating through all equipment and add each matching item to the list.

Testing

Added testMultipleEquipmentTemporaryBonus() test case that:

• Creates 4 different weapons (Dagger, Longsword, Sling, Longbow) with various types (Martial/Simple)
• Applies a temporary bonus that matches all weapon types
• Verifies that all 4 weapons are returned in the applicable equipment list
• Confirms each weapon is present by name

Impact

Users will now see all their eligible weapons when applying temporary bonuses, allowing them to choose which specific weapon to apply the bonus to. This is especially important for characters carrying multiple weapons of different types.

Files changed: 2 files, 2 lines modified, 43 lines added (test)

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• java.freehep.org
  • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED -Xmx4096m -Dfile.encoding=UTF-8 -Duser.country -Duser.language=en -Duser.variant -cp /home/REDACTED/.gradle/wrapper/dists/gradle-8.10.2-all/7iv73wktx1xtkvlq19urqw1wm/gradle-8.10.2/lib/gradle-daemon-main-8.10.2.jar -javaagent:/home/REDACTED/.gradle/wrapper/dists/gradle-8.10.2-all/7iv73wktx1xtkvlq19urqw1wm/gradle-8.10.2/lib/agents/gradle-instrumentation-agent-8.10.2.jar org.gradle.launcher.daemon.bootstrap.GradleDaemon 8.10.2 (dns block)
• pc-gen.org
• repository.jboss.org
  • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED -Xmx4096m -Dfile.encoding=UTF-8 -Duser.country -Duser.language=en -Duser.variant -cp /home/REDACTED/.gradle/wrapper/dists/gradle-8.10.2-all/7iv73wktx1xtkvlq19urqw1wm/gradle-8.10.2/lib/gradle-daemon-main-8.10.2.jar -javaagent:/home/REDACTED/.gradle/wrapper/dists/gradle-8.10.2-all/7iv73wktx1xtkvlq19urqw1wm/gradle-8.10.2/lib/agents/gradle-instrumentation-agent-8.10.2.jar org.gradle.launcher.daemon.bootstrap.GradleDaemon 8.10.2 (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Original prompt

---

This section details on the original issue you should resolve

<issue_title>"Select the target of the bonus"</issue_title>
<issue_description>6.09.07 Selecting the "Temporary Bonuses" Tab and selecting any Temp (i.e. Magic Weapon), I see only one weapon when there are many, how to see all the weapons in the "Select the target of the bonus"?
and where do they need to be on the character to see the all the weapons, i.e. Carried, Primary Hand, Secondary Hand, Equipped, etc....</issue_description>

Comments on the Issue (you are @copilot in this section)

@LegacyKing @mertonmonk Something is acting up with the TEMPBONUS tag - the internal code chooser is only taking the first alphabetically valid target. @LegacyKing It might be random - I tried sling and longbow with gravity bow and the selection was only sling.

Fixes #7418

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[karianna]

@mertonmonk Does this make sense to you?

[mertonmonk]

Yeah - I'm not sure why you'd want to break out of the Equipment level loop just because you found a reason to add one item based on its bonus passing the condition. I think getting rid of FINDEQ and just having a normal break is the right solution.

[mertonmonk]

@karianna please approve this so we can merge the code.

[Athinar]

Okay now what? download?

[LegacyKing]

Just an FYI this breaks the build test --

C:\Projects\PCGen\pcgen_608\code\src\test\pcgen\core\bonus\TempBonusTest.java:144: error: incompatible types: boolean cannot be converted to String assertTrue(weaponNames.contains("Dagger"), "List should contain Dagger"); ^ C:\Projects\PCGen\pcgen_608\code\src\test\pcgen\core\bonus\TempBonusTest.java:145: error: incompatible types: boolean cannot be converted to String assertTrue(weaponNames.contains("Longsword"), "List should contain Longsword"); ^ C:\Projects\PCGen\pcgen_608\code\src\test\pcgen\core\bonus\TempBonusTest.java:146: error: incompatible types: boolean cannot be converted to String assertTrue(weaponNames.contains("Sling"), "List should contain Sling"); ^ C:\Projects\PCGen\pcgen_608\code\src\test\pcgen\core\bonus\TempBonusTest.java:147: error: incompatible types: boolean cannot be converted to String assertTrue(weaponNames.contains("Longbow"), "List should contain Longbow");

[LegacyKing]

@karianna @mertonmonk -- this commit is bad and will block any release

[LegacyKing]

FYI, I've reverted this from my build version for 6.08

I'll leave in master for now but we need to fix the assert slowtest or this code.

[mertonmonk]

@LegacyKing I'll take a look at it over the weekend - was it the java code change or the test code additions that broke the build?

[mertonmonk]

@LegacyKing If you remove the changes to the TempBonusTest class, the build will work. Removing FINDEQ from the for loop is the correct fix for the bug - the test code was bad.

[Athinar]

So do I need to download the newest 6.09?

[Athinar]

Any luck with 6.09?

[karianna]

See #7489 - we just need to land that and try to push out a new release