Skip to content

Commit

Permalink
Better fix for the previous fix
Browse files Browse the repository at this point in the history
  • Loading branch information
richardwilkes committed Jul 18, 2024
1 parent 6e74995 commit 5671eda
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 5 deletions.
4 changes: 1 addition & 3 deletions model/gurps/weapon.go
Original file line number Diff line number Diff line change
Expand Up @@ -548,9 +548,7 @@ func (w *Weapon) collectWeaponBonuses(dieCount int, tooltip *xio.ByteBuffer, all
var bestDef *SkillDefault
best := fxp.Min
for _, one := range w.Defaults {
// Need the nil check here, as an entry in the defaults list could be nil due if this weapon pointer was
// obtained via a stale owner reference during an edit operation.
if one != nil && one.SkillBased() {
if one.SkillBased() {
if level := one.SkillLevelFast(entity, false, nil, true); best < level {
best = level
bestDef = one
Expand Down
14 changes: 12 additions & 2 deletions ux/defaults_panel.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,10 @@ func newDefaultsPanel(entity *gurps.Entity, defaults *[]*gurps.SkillDefault) *de
addButton := unison.NewSVGButton(svg.CircledAdd)
addButton.ClickCallback = func() {
def := &gurps.SkillDefault{DefaultType: lastDefaultTypeUsed}
*p.defaults = slices.Insert(*p.defaults, 0, def)
// See the comment for the delete button as to why we don't just use slices.Insert here.
defs := make([]*gurps.SkillDefault, len(*p.defaults)+1)
defs[0] = def
copy(defs[1:], *p.defaults)
p.insertDefaultsPanel(1, def)
MarkRootAncestorForLayoutRecursively(p)
MarkModified(p)
Expand All @@ -81,7 +84,14 @@ func (p *defaultsPanel) insertDefaultsPanel(index int, def *gurps.SkillDefault)
deleteButton := unison.NewSVGButton(svg.Trash)
deleteButton.ClickCallback = func() {
if i := slices.IndexFunc(*p.defaults, func(elem *gurps.SkillDefault) bool { return elem == def }); i != -1 {
*p.defaults = slices.Delete(*p.defaults, i, i+1)
// Cannot use this here: *p.defaults = slices.Delete(*p.defaults, i, i+1)
// because it will cause a panic elsewhere due to some code not having the owner properly updated. This
// causes the original slice to be looked at, which now has a nil at its end, which causes problems in code
// that expects no nils. So... we do it the more expensive and verbose way.
defs := make([]*gurps.SkillDefault, len(*p.defaults)-1)
copy(defs, (*p.defaults)[:i])
copy(defs[i:], (*p.defaults)[i+1:])
*p.defaults = defs
}
panel.RemoveFromParent()
MarkRootAncestorForLayoutRecursively(p)
Expand Down

0 comments on commit 5671eda

Please sign in to comment.