Skip to content

Fix bug where dummy weapon bps are assigned to non-dummy weapons - #6882

Merged
lL1l1 merged 9 commits into
FAForever:developfrom
lL1l1:Fix/blueprint-ModWeapon
Jul 24, 2026
Merged

Fix bug where dummy weapon bps are assigned to non-dummy weapons#6882
lL1l1 merged 9 commits into
FAForever:developfrom
lL1l1:Fix/blueprint-ModWeapon

Conversation

@lL1l1

@lL1l1 lL1l1 commented Jun 29, 2025

Copy link
Copy Markdown
Contributor

Issue

Discovered when I was helping a new modder on Discord.

If a dummy weapon is before a non-dummy weapon in the unit blueprint's Weapon table, then the engine assigns the dummy weapon's bp to the non-dummy weapon. This is because the engine counts n real weapons, and simply assigns indices 1-n to weapons 1-n. If a dummy weapon is in 1-n then it gets assigned to a real weapon which breaks that weapon.

This is an easy issue to run into if using the ModWeapon field in blueprints, which by default places weapons at the end of the weapons table.
It can also happen if someone doesn't know about this issue and places a dummy weapon in the middle by chance.

Description of the proposed changes

In post-mod weapon processing, detect if non-dummy weapons are placed after dummy weapons, and re-order the dummy weapons to the end of the array.

Also check for dummy weapons in the weapon merge function.

Testing done on the proposed changes

The error from the test commit disappears after applying the changes.

Checklist

lL1l1 added 5 commits June 28, 2025 20:36
Credit to Laserstorm on the faf discord for the initial code which I fixed and cleaned up for testing here. Error message:
`ERROR: No RackBones table specified, aborting weapon setup.  Weapon: TeleportWeapon on Unit: uel0001`
The UEF ACU no longer errors upon spawning
@lL1l1
lL1l1 requested review from Garanas and Hdt80bro June 29, 2025 06:59
@lL1l1 lL1l1 added type: bug area: engine related to engine bugs/limitations feature: mod compatibility related to FAF compatibility with mods labels Jun 29, 2025
lL1l1 added 2 commits June 29, 2025 00:41
also a small refactor

This avoids the warning being given later on unnecessarily
@lL1l1
lL1l1 marked this pull request as ready for review June 29, 2025 08:28
@lL1l1
lL1l1 requested a review from speed2CZ June 29, 2025 08:28

@Garanas Garanas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it not be better to throw a warning and let the author fix it?

I'm not sure if re-ordering the weapons is the best thing to do here. It would make a unit 'work' for FAF, but not for Steam or LOUD for example.

@lL1l1

lL1l1 commented Jul 8, 2025

Copy link
Copy Markdown
Contributor Author

It already gives a warning. I agree that the author should learn and fix their mistakes, improving compatibility with other environments, if they do a custom implementation of the weapons table (as simple as writing it in a blueprint). But the MergeWeaponByLabel feature does need this behavior for it to be easy to use, so I'll keep it in that function.

@lL1l1

lL1l1 commented Jul 8, 2025

Copy link
Copy Markdown
Contributor Author

It's unlikely, but auto-fixing it could also break mods that hacked around the bug or fix old mods that had the bug. Would just have to see if there are any new warnings when I play.

@Garanas

Garanas commented Jul 14, 2025

Copy link
Copy Markdown
Member

Maybe @The-Balthazar has an idea how often that can happen in practice, based on his own sim mods and unit packs.

@BlackYps BlackYps removed this from the Development Iteration III of 2025 milestone Aug 15, 2025
@The-Balthazar

Copy link
Copy Markdown
Contributor

It probably doesn't help that MergeWeaponByLabel was written by someone who assumes all weapons have unique labels.

    for i, w in weaponTable do
        if w.Label == label then
            weaponTable[i] = BlueprintMerged(w, newBp)
            return
        end

Should probably not return there if you want a label merge to apply to each of a given label assuming merges are unique labels only, or it needs a full rewrite to keep a count of relative instances of given labels in the base vs the merge.

Did the dummies in this case have labels that matched other later indexed weapons?

@lL1l1

lL1l1 commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

I addressed your concern in #7191.

The dummies didn't have labels that matched other weapons. The modder was adding a brand new weapon to the end of the UEF ACU's weapons table, but since the UEF ACU has a death nuke dummy weapon, that was causing the bug described in the PR.

@lL1l1
lL1l1 merged commit b026f7a into FAForever:develop Jul 24, 2026
5 checks passed
@lL1l1
lL1l1 deleted the Fix/blueprint-ModWeapon branch July 24, 2026 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: engine related to engine bugs/limitations feature: mod compatibility related to FAF compatibility with mods type: bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants