diff --git a/code/src/java/pcgen/cdom/choiceset/SpellCasterChoiceSet.java b/code/src/java/pcgen/cdom/choiceset/SpellCasterChoiceSet.java index 6a92e9af275..cfff07680ad 100644 --- a/code/src/java/pcgen/cdom/choiceset/SpellCasterChoiceSet.java +++ b/code/src/java/pcgen/cdom/choiceset/SpellCasterChoiceSet.java @@ -39,12 +39,15 @@ /** * A SpellCasterChoiceSet contains references to PCClass Objects. - * + * * The contents of a SpellCasterChoiceSet is defined at construction of the * SpellCasterChoiceSet. The contents of a SpellCasterChoiceSet is fixed, and * will not vary by the PlayerCharacter used to resolve the * SpellCasterChoiceSet. */ +@SuppressFBWarnings(value = "EQ_DOESNT_OVERRIDE_EQUALS", + justification = "Parent ChoiceSet.equals compares setName + pcs; pcs here is the typePCS passed to super, " + + "which captures the relevant identity. Overriding equals here would break symmetry with the parent.") public class SpellCasterChoiceSet extends ChoiceSet implements PrimitiveChoiceSet { @@ -273,49 +276,11 @@ public Set getSet(PlayerCharacter pc) return returnSet; } - @Override - public int hashCode() - { - return (types == null ? 0 : types.hashCode() * 29) + (primitives == null ? 0 : primitives.hashCode()); - } - - @Override - public boolean equals(Object obj) - { - if (obj == this) - { - return true; - } - if (obj instanceof SpellCasterChoiceSet other) - { - if (types == null) - { - if (other.types != null) - { - return false; - } - } - else - { - if (!types.equals(other.types)) - { - return false; - } - } - if (primitives == null) - { - return other.primitives == null; - } - return primitives.equals(other.primitives); - } - return false; - } - /** * Returns the GroupingState for this SpellCasterChoiceSet. The * GroupingState indicates how this SpellCasterChoiceSet can be combined * with other PrimitiveChoiceSets. - * + * * @return The GroupingState for this SpellCasterChoiceSet. */ @Override diff --git a/code/src/java/pcgen/output/actor/DescriptionActor.java b/code/src/java/pcgen/output/actor/DescriptionActor.java index 1a403aed28b..5c7ccb77a95 100644 --- a/code/src/java/pcgen/output/actor/DescriptionActor.java +++ b/code/src/java/pcgen/output/actor/DescriptionActor.java @@ -69,7 +69,14 @@ public TemplateModel process(CharID id, PObject d) throws TemplateModelException return FacetLibrary.getFacet(ObjectWrapperFacet.class).wrap(id, Constants.EMPTY_STRING); } PlayerCharacterTrackingFacet charStore = SpringHelper.getBean(PlayerCharacterTrackingFacet.class); - PlayerCharacter aPC = charStore.getPC(id); + PlayerCharacter aPC = (charStore == null) ? null : charStore.getPC(id); + if (aPC == null) + { + // SpringHelper.getBean returns null when no bean is registered, and + // PlayerCharacterTrackingFacet.getPC documents that null may be returned + // in unit-test paths where no PC has been associated with the CharID. + return FacetLibrary.getFacet(ObjectWrapperFacet.class).wrap(id, Constants.EMPTY_STRING); + } final StringBuilder buf = new StringBuilder(250); boolean needSpace = false; for (final Description desc : theBenefits) diff --git a/code/src/test/pcgen/cdom/choiceset/SpellCasterChoiceSetEqualsTest.java b/code/src/test/pcgen/cdom/choiceset/SpellCasterChoiceSetEqualsTest.java new file mode 100644 index 00000000000..d34683fda4e --- /dev/null +++ b/code/src/test/pcgen/cdom/choiceset/SpellCasterChoiceSetEqualsTest.java @@ -0,0 +1,121 @@ +/* + * Copyright 2026 Vest + * + * This library is free software; you can redistribute it and/or modify it under + * the terms of the GNU Lesser General Public License as published by the + * Free Software Foundation; either version 2.1 of the License, or (at your + * option) any later version. + * + * This library is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this library; if not, write to the Free Software Foundation, + * Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ +package pcgen.cdom.choiceset; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Collections; +import java.util.List; +import java.util.Objects; +import java.util.Set; + +import pcgen.cdom.base.ChoiceSet; +import pcgen.cdom.base.PrimitiveChoiceSet; +import pcgen.cdom.enumeration.GroupingState; +import pcgen.core.PCClass; +import pcgen.core.PlayerCharacter; + +import org.junit.jupiter.api.Test; + +/** + * Regression test for the SpotBugs EQ_OVERRIDING_EQUALS_NOT_SYMMETRIC finding + * that previously affected {@link SpellCasterChoiceSet}. Before the fix the + * subclass overrode {@code equals} to require {@code SpellCasterChoiceSet} on + * the other side, which broke symmetry with {@link ChoiceSet#equals(Object)}. + * The fix removes that override and inherits the parent equality. + */ +class SpellCasterChoiceSetEqualsTest +{ + + /** + * Minimal stub PrimitiveChoiceSet identified by name; equals compares names. + * Suitable for use as both the parent ChoiceSet's pcs and the subclass's + * typePCS so that both ChoiceSets share the same {@code pcs} reference. + */ + private static PrimitiveChoiceSet stubPcs(String name) + { + return new PrimitiveChoiceSet<>() + { + @Override + public Class getChoiceClass() + { + return PCClass.class; + } + + @Override + public String getLSTformat(boolean useAny) + { + return name; + } + + @Override + public Set getSet(PlayerCharacter pc) + { + return Collections.emptySet(); + } + + @Override + public GroupingState getGroupingState() + { + return GroupingState.ANY; + } + + @Override + public boolean equals(Object o) + { + return (o instanceof PrimitiveChoiceSet) && name.equals(((PrimitiveChoiceSet) o).getLSTformat(true)); + } + + @Override + public int hashCode() + { + return name.hashCode(); + } + }; + } + + @Test + void parentAndSpellCasterAreSymmetricWhenSharingNameAndPcs() + { + PrimitiveChoiceSet sharedPcs = stubPcs("CASTERS"); + + // Parent ChoiceSet, name + pcs only — the identity ChoiceSet.equals checks. + ChoiceSet plainSet = new ChoiceSet<>("SPELLCASTER", sharedPcs); + + // SpellCasterChoiceSet that passes the same pcs to super(...) so the + // parent-visible identity matches. + SpellCasterChoiceSet casterSet = + new SpellCasterChoiceSet(null, List.of(), sharedPcs, null); + + // Symmetry of equals: both directions must agree (used to fail). + assertEquals(plainSet.equals(casterSet), casterSet.equals(plainSet), + "ChoiceSet.equals(SpellCasterChoiceSet) must equal SpellCasterChoiceSet.equals(ChoiceSet)"); + + // And the parent-defined identity says they are equal. + assertTrue(plainSet.equals(casterSet), "parent considers them equal (same setName + pcs)"); + assertTrue(casterSet.equals(plainSet), "subclass must agree (symmetry)"); + + // Objects.equals goes through the receiver-side equals; verify both routes. + assertEquals(Objects.equals(plainSet, casterSet), Objects.equals(casterSet, plainSet)); + + // hashCode contract: equal objects must hash the same. + assertEquals(plainSet.hashCode(), casterSet.hashCode(), + "equal ChoiceSet/SpellCasterChoiceSet must hash equally"); + } +}