Skip to content

update annotations for engine runtime defined fields#7171

Open
Lightningbulb2 wants to merge 1 commit into
FAForever:developfrom
Lightningbulb2:runtime_annotation_overhaul
Open

update annotations for engine runtime defined fields#7171
Lightningbulb2 wants to merge 1 commit into
FAForever:developfrom
Lightningbulb2:runtime_annotation_overhaul

Conversation

@Lightningbulb2

@Lightningbulb2 Lightningbulb2 commented Jul 6, 2026

Copy link
Copy Markdown

WIP PR

Description of the proposed changes

• This adds a generic alias annotation that can be applied to field annotations to stop language server errors for fields that will only be defined during runtime on the engine side.

• makes changes to class.lua to prevent errors from moho stuff in the FA class parameters

Additional context

This is in preparation for the WIP updated language server

This PR's content was written entirely by hand besides the class.lua changes.

Checklist

Summary by CodeRabbit

  • New Features

    • Improved editor support for many game data/configuration fields, especially in blueprints, UI, AI, and lobby settings.
    • Added a reusable way to mark engine-filled fields, reducing false missing-field warnings and improving autocomplete.
  • Bug Fixes

    • Fixed several Lua language-server annotation issues that could cause noisy type errors in project files.
    • Cleaned up a few formatting and documentation markers to keep tooling output stable.
  • Tests

    • Updated test file section markers for clearer organization.

@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR introduces a global EngineDefinedField<T> nullable type alias and applies it broadly across engine and game Lua files to re-annotate fields populated by the engine at runtime, spanning blueprints, UI, AI, sim modules, and class generics. Also includes minor tooling settings and test region-marker adjustments.

Changes

EngineDefinedField annotation rollout

Layer / File(s) Summary
EngineDefinedField alias and class generics
lua/system/class.lua
Adds EngineDefinedField<T> = T | nil alias and removes : fa-class generic constraint from several class constructor docblocks.
Core Blueprint annotations
engine/AnnotationCommon.lua, engine/Core/Blueprints/Blueprint.lua, engine/Core/Blueprints/EntityBlueprint.lua, engine/Core/Blueprints/MeshBlueprint.lua
Wraps multiple blueprint fields (collision, lifebar, mesh, hitbox, sound) with EngineDefinedField<...>.
UnitBlueprint annotations
engine/Core/Blueprints/UnitBlueprint.lua, units/XSB1104/XSB1104_unit.bp
Retypes numerous nested UnitBlueprint sections (AI, Display, Economy, General, Physics, Defense, Tarmac, Intel) to EngineDefinedField<...> and disables missing-fields diagnostic on a unit blueprint file.
WeaponBlueprint annotations
engine/Core/Blueprints/WeaponBlueprint.lua
Wraps aim/target, beam, muzzle, projectile, rack, and turret fields with EngineDefinedField<...>.
Engine user/control annotations
engine/User/CLobby.lua, engine/User/CMauiControl.lua
Wraps lobby launch configuration and control-methods fields with EngineDefinedField<...>.
AI/aibrains annotations
lua/AI/OpAI/BaseManager.lua, lua/aibrains/..., lua/AI/AIBaseTemplates/RushMainNaval.lua, lua/AI/AIBuilders/AILandAttackBuilders.lua, lua/system/GlobalBaseTemplate.lua, lua/system/GlobalBuilderTemplate.lua
Retypes AI base/structure/platoon debug info, task, builder-group/builder template fields; adjusts FirstBaseFunction return signature and removes a @return string.
Sim module annotations
lua/TerrainTypes.lua, lua/aibrain.lua, lua/defaultcomponents.lua, lua/shared/NavGenerator.lua, lua/shared/Profiler.lua, lua/sim/*
Wraps terrain, veterancy, nav debug, profiler, marker, and unit/weapon fields with EngineDefinedField<...>.
UI module annotations
lua/maui/checkbox.lua, lua/maui/control.lua, lua/ui/game/*, lua/ui/lobby/*, lua/ui/uiutil.lua
Wraps chat, order, ping, reclaim, lobby option, and control fields with EngineDefinedField<...>.
Tooling config, docs, and test markers
.vscode/settings.json, lua/system/utils.lua, tests/blueprint/unit.spec.lua
Updates Lua runtime version/workspace settings, refines doc comments, and adds missing --#endregion markers.

Estimated code review effort: 2 (Simple) | ~15 minutes

Possibly related PRs

  • FAForever/fa#7137: Both PRs touch lua/modules/ui/lobby/lobbyOptions.lua, updating GameOptions.CommonArmy type annotations.

Suggested labels: area: documentation

Suggested reviewers: BlackYps, Garanas, lL1l1

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding engine runtime-defined field annotations.
Description check ✅ Passed The PR includes the main change description and context, with only the testing section left unfilled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 6

♻️ Duplicate comments (1)
engine/Core/Blueprints/Blueprint.lua (1)

3-5: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Same generic-instantiation space concern as flagged in AnnotationCommon.lua.

Note the internal inconsistency in this very file: the alias is declared as EngineDefinedField<T> (no space) at line 4, but every usage site (BlueprintId, Source, LodCutoff) uses EngineDefinedField <...> with a space. Since the LuaCats examples for generic instantiation use no space, this discrepancy is worth resolving consistently across the file.

Also applies to: 16-21, 35-35

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@engine/Core/Blueprints/Blueprint.lua` around lines 3 - 5, The generic alias
usage in Blueprint.lua is inconsistent: EngineDefinedField is declared without a
space, but BlueprintId, Source, and LodCutoff instantiate it with a space.
Update those type annotations to use the same generic instantiation style
consistently across the file, matching the alias name in EngineDefinedField and
the other related annotations in this module.
🧹 Nitpick comments (2)
lua/TerrainTypes.lua (1)

134-143: 📐 Maintainability & Code Quality | 🔵 Trivial

Keep script-owned terrain fields plain.

These values are defined in TerrainTypes.lua, not engine-populated. Wrapping them in EngineDefinedField makes the contract look nullable for no functional gain and weakens type checking for consumers of TerrainTypes.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lua/TerrainTypes.lua` around lines 134 - 143, The script-owned terrain fields
in TerrainTypes should not be marked as engine-populated. Update the
TerrainTypes declarations for Style, Slippery, Bumpiness, Blocking, FXIdle,
FXImpact, FXLayerChange, FXMotionChange, FXMovement, and FXOther to use plain
field types instead of EngineDefinedField, keeping the existing concrete type
annotations so consumers get accurate non-nullable typing.
lua/defaultcomponents.lua (1)

600-602: 📐 Maintainability & Code Quality | 🔵 Trivial

Keep veterancy state non-nullable.

VetDamage, VetDamageTaken, and VetInstigators are initialized in OnCreate and used as required state everywhere below. Marking them EngineDefinedField obscures that invariant and adds avoidable nil noise for downstream consumers.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lua/defaultcomponents.lua` around lines 600 - 602, The veterancy fields are
required state, not optional engine-defined values, so the annotations in
defaultcomponents.lua should reflect that invariant. Update the field
declarations for VetDamage, VetDamageTaken, and VetInstigators to use
non-nullable required typing consistent with how OnCreate initializes them and
how the veterancy code consumes them below. Keep the existing symbol names
unchanged and remove the nullable/EngineDefinedField signaling so downstream
users no longer need nil checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@engine/User/CLobby.lua`:
- Around line 46-47: The type annotations in CLobby are using a spaced alias
form that prevents LuaLS from resolving the fields correctly. Update the Ratings
and Divisions annotations in the CLobby class comment to use
EngineDefinedField<...> consistently, keeping the alias attached to the generic
type so the fields remain properly typed and recognized.

In `@lua/AI/AIBuilders/AILandAttackBuilders.lua`:
- Around line 28-30: The `LandAttackCondition` annotation currently marks
`aiBrain` as optional even though the function dereferences it immediately and
the builder path already passes `self.Brain`. Update the `---@param aiBrain?`
declaration in `AILandAttackBuilders.lua` to make `aiBrain` required so the
documented contract matches the actual usage of `LandAttackCondition` and its
caller.

In `@lua/aibrains/platoons/platoon-base.lua`:
- Around line 10-13: Update the platoon debug-info flow so GetDebugInfo no
longer assumes self.DebugMessages exists; in platoon-base.lua, make GetDebugInfo
sort a safe local fallback like self.DebugMessages or {} or ensure the field is
initialized before any access. Keep LogDebug and LogWarning compatible with lazy
creation, and preserve PlatoonInfo as nullable so the missing-debug-messages
path remains valid.

In `@lua/sim/Unit.lua`:
- Line 161: Update the `Unit.Sync` annotation in `Unit.lua` so it matches the
terminal value set by `OnDestroy`; the field is currently typed only as the sync
table, but `self.Sync` is also assigned `false`, so expand the `---@field Sync`
type to include `false` while keeping the existing `{ id: string, army: Army }`
shape.

In `@lua/ui/lobby/lobbyOptions.lua`:
- Line 22: The TeamSpawn annotation in lobbyOptions.lua is missing a valid lobby
option value that teamOptions already exposes. Update the EngineDefinedField
union for TeamSpawn to include penguin_autobalance so the type matches the
actual values used by teamOptions and LuaLS stops flagging it. Keep the change
scoped to the TeamSpawn field annotation and ensure it stays aligned with the
option list in this file.

In `@units/XSB1104/XSB1104_unit.bp`:
- Around line 1-5: The diagnostic suppression comment in XSB1104_unit.bp is
using the wrong comment prefix, so LuaLS ignores it; update the existing
missing-fields directive near the top of the file to use the standard three-dash
form with the same symbol name, ensuring the suppression is recognized by the
language server.

---

Duplicate comments:
In `@engine/Core/Blueprints/Blueprint.lua`:
- Around line 3-5: The generic alias usage in Blueprint.lua is inconsistent:
EngineDefinedField is declared without a space, but BlueprintId, Source, and
LodCutoff instantiate it with a space. Update those type annotations to use the
same generic instantiation style consistently across the file, matching the
alias name in EngineDefinedField and the other related annotations in this
module.

---

Nitpick comments:
In `@lua/defaultcomponents.lua`:
- Around line 600-602: The veterancy fields are required state, not optional
engine-defined values, so the annotations in defaultcomponents.lua should
reflect that invariant. Update the field declarations for VetDamage,
VetDamageTaken, and VetInstigators to use non-nullable required typing
consistent with how OnCreate initializes them and how the veterancy code
consumes them below. Keep the existing symbol names unchanged and remove the
nullable/EngineDefinedField signaling so downstream users no longer need nil
checks.

In `@lua/TerrainTypes.lua`:
- Around line 134-143: The script-owned terrain fields in TerrainTypes should
not be marked as engine-populated. Update the TerrainTypes declarations for
Style, Slippery, Bumpiness, Blocking, FXIdle, FXImpact, FXLayerChange,
FXMotionChange, FXMovement, and FXOther to use plain field types instead of
EngineDefinedField, keeping the existing concrete type annotations so consumers
get accurate non-nullable typing.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 85a1299e-596b-4e7d-be26-1444a9fae4e6

📥 Commits

Reviewing files that changed from the base of the PR and between 827ec16 and f60aa98.

📒 Files selected for processing (50)
  • .vscode/settings.json
  • engine/AnnotationCommon.lua
  • engine/Core/Blueprints/Blueprint.lua
  • engine/Core/Blueprints/EntityBlueprint.lua
  • engine/Core/Blueprints/MeshBlueprint.lua
  • engine/Core/Blueprints/UnitBlueprint.lua
  • engine/Core/Blueprints/WeaponBlueprint.lua
  • engine/User/CLobby.lua
  • engine/User/CMauiControl.lua
  • lua/AI/AIBaseTemplates/RushMainNaval.lua
  • lua/AI/AIBuilders/AILandAttackBuilders.lua
  • lua/AI/OpAI/BaseManager.lua
  • lua/TerrainTypes.lua
  • lua/aibrain.lua
  • lua/aibrains/managers/base-manager.lua
  • lua/aibrains/managers/structure-manager.lua
  • lua/aibrains/platoons/platoon-base.lua
  • lua/aibrains/tasks/task.lua
  • lua/aibrains/templates/base/base-template.lua
  • lua/aibrains/templates/builder-groups/builder-group-template.lua
  • lua/aibrains/templates/builders/builder-template.lua
  • lua/defaultcomponents.lua
  • lua/maui/checkbox.lua
  • lua/maui/control.lua
  • lua/shared/ChatPayload.lua
  • lua/shared/NavGenerator.lua
  • lua/shared/Profiler.lua
  • lua/sim/BuilderManager.lua
  • lua/sim/MarkerUtilities.lua
  • lua/sim/NavGenerator.lua
  • lua/sim/Unit.lua
  • lua/sim/commands/shared.lua
  • lua/sim/units/uef/TPodTowerUnit.lua
  • lua/sim/weapon.lua
  • lua/system/GlobalBaseTemplate.lua
  • lua/system/GlobalBuilderTemplate.lua
  • lua/system/class.lua
  • lua/system/utils.lua
  • lua/ui/game/chat/ChatCommandHintInterface.lua
  • lua/ui/game/chat/ChatModel.lua
  • lua/ui/game/commandmode.lua
  • lua/ui/game/hotkeys/context-based-templates-data.lua
  • lua/ui/game/orders.lua
  • lua/ui/game/ping.lua
  • lua/ui/game/reclaim.lua
  • lua/ui/lobby/autolobby/AutolobbyController.lua
  • lua/ui/lobby/lobbyOptions.lua
  • lua/ui/uiutil.lua
  • tests/blueprint/unit.spec.lua
  • units/XSB1104/XSB1104_unit.bp

Comment thread engine/User/CLobby.lua
Comment on lines +46 to +47
---@field Ratings EngineDefinedField <table<string, number>>
---@field Divisions EngineDefinedField <table<string, string>>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Does lua-language-server require no whitespace between a generic alias name and its angle brackets for instantiation, e.g. MyAliasvsMyAlias ?

💡 Result:

The lua-language-server generally requires that there be no whitespace between a generic alias name and its angle brackets when defining or instantiating types [1]. In LuaLS, generic aliases are defined using syntax such as ---@alias MyAlias<K> { [K]: true } [1]. Official documentation and community examples consistently place the angle brackets immediately after the identifier without spaces (e.g., MyAlias<T>) [2][3][4][1]. Inserting a space (e.g., MyAlias <T>) is generally not supported by the parser and may result in the language server failing to correctly recognize or resolve the generic type [5]. The parser expects the identifier and the opening angle bracket to be adjacent to correctly identify the generic pattern [6][1].

Citations:


🏁 Script executed:

sed -n '1,120p' engine/User/CLobby.lua

Repository: FAForever/fa

Length of output: 5682


🏁 Script executed:

sed -n '1,40p' lua/system/class.lua

Repository: FAForever/fa

Length of output: 1734


Remove the space before EngineDefinedField<...>
Use EngineDefinedField<...> in both annotation blocks here; EngineDefinedField <...> can stop LuaLS from instantiating the alias, leaving these fields unresolved and defeating the nilable typing.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@engine/User/CLobby.lua` around lines 46 - 47, The type annotations in CLobby
are using a spaced alias form that prevents LuaLS from resolving the fields
correctly. Update the Ratings and Divisions annotations in the CLobby class
comment to use EngineDefinedField<...> consistently, keeping the alias attached
to the generic type so the fields remain properly typed and recognized.

Comment on lines +28 to +30
---@param aiBrain? AIBrain
---@param locationType LocationType
---@param targetNumber number

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== File outline ==\n'
ast-grep outline lua/AI/AIBuilders/AILandAttackBuilders.lua --view expanded || true

printf '\n== Relevant lines ==\n'
cat -n lua/AI/AIBuilders/AILandAttackBuilders.lua | sed -n '1,120p'

printf '\n== References to LandAttackCondition ==\n'
rg -n "LandAttackCondition" -S lua || true

Repository: FAForever/fa

Length of output: 8730


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== Condition/function invocation sites ==\n'
rg -n "BuilderConditions|Condition\(|\bLandAttackCondition\b|function .*Condition" lua -S || true

printf '\n== Optional aiBrain annotations elsewhere ==\n'
rg -n "`@param` aiBrain\?" lua -S || true

Repository: FAForever/fa

Length of output: 46156


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== lua/sim/Builder.lua (relevant section) ==\n'
cat -n lua/sim/Builder.lua | sed -n '40,90p;200,260p'

printf '\n== lua/sim/BrainConditionsMonitor.lua (relevant section) ==\n'
cat -n lua/sim/BrainConditionsMonitor.lua | sed -n '170,240p'

Repository: FAForever/fa

Length of output: 7555


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== CheckKeyedCondition and function invocation path ==\n'
rg -n "CheckKeyedCondition|FunctionConditions|ConditionParameters|CheckCondition = function|Condition.*Function" lua/sim lua/AI -S || true

printf '\n== Relevant snippets ==\n'
grep -n "CheckKeyedCondition\|FunctionConditions\|ConditionParameters" -n lua/sim/BrainConditionsMonitor.lua lua/sim/*.lua 2>/dev/null || true

Repository: FAForever/fa

Length of output: 5314


Make aiBrain required here. LandAttackCondition dereferences aiBrain immediately, and the builder path passes self.Brain, so ---@param aiBrain? advertises a nil-safe contract this function doesn’t support.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lua/AI/AIBuilders/AILandAttackBuilders.lua` around lines 28 - 30, The
`LandAttackCondition` annotation currently marks `aiBrain` as optional even
though the function dereferences it immediately and the builder path already
passes `self.Brain`. Update the `---@param aiBrain?` declaration in
`AILandAttackBuilders.lua` to make `aiBrain` required so the documented contract
matches the actual usage of `LandAttackCondition` and its caller.

Comment on lines +10 to +13
---@field EntityId EngineDefinedField <EntityId>
---@field BlueprintId EngineDefinedField <BlueprintId>
---@field Position EngineDefinedField <Vector>
---@field PlatoonInfo EngineDefinedField <{ DebugMessages: string[], PlatoonName: string, StateName: string }>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '--- platoon-base.lua outline ---\n'
ast-grep outline lua/aibrains/platoons/platoon-base.lua --view expanded || true

printf '\n--- relevant symbols ---\n'
rg -n "DebugMessages|GetDebugInfo|LogDebug|LogWarning|PlatoonInfo" lua/aibrains/platoons/platoon-base.lua

printf '\n--- file excerpt ---\n'
cat -n lua/aibrains/platoons/platoon-base.lua | sed -n '1,260p'

Repository: FAForever/fa

Length of output: 241


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- platoon-base.lua outline ---'
ast-grep outline lua/aibrains/platoons/platoon-base.lua --view expanded || true

printf '%s\n' ''
printf '%s\n' '--- relevant symbols ---'
rg -n "DebugMessages|GetDebugInfo|LogDebug|LogWarning|PlatoonInfo" lua/aibrains/platoons/platoon-base.lua

printf '%s\n' ''
printf '%s\n' '--- file excerpt ---'
cat -n lua/aibrains/platoons/platoon-base.lua | sed -n '1,260p'

Repository: FAForever/fa

Length of output: 11261


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- GetDebugInfo excerpt ---'
sed -n '540,590p' lua/aibrains/platoons/platoon-base.lua

printf '%s\n' ''
printf '%s\n' '--- call sites for GetDebugInfo ---'
rg -n "\bGetDebugInfo\b" -S lua .

Repository: FAForever/fa

Length of output: 3387


Guard GetDebugInfo against missing DebugMessages

LogDebug/LogWarning create self.DebugMessages lazily, but GetDebugInfo still sorts self.DebugMessages unconditionally. If debug info is requested before the first log entry, this can crash; initialize it earlier or sort a local self.DebugMessages or {}. The PlatoonInfo field should stay nullable to match that path.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lua/aibrains/platoons/platoon-base.lua` around lines 10 - 13, Update the
platoon debug-info flow so GetDebugInfo no longer assumes self.DebugMessages
exists; in platoon-base.lua, make GetDebugInfo sort a safe local fallback like
self.DebugMessages or {} or ensure the field is initialized before any access.
Keep LogDebug and LogWarning compatible with lazy creation, and preserve
PlatoonInfo as nullable so the missing-debug-messages path remains valid.

Comment thread lua/sim/Unit.lua
---@field PlatoonHandle? Platoon
---@field tickIssuedShieldRepair number? # Used by shields to keep track of when this unit's guards were ordered to start shield repair instantly
---@field Sync { id: string, army: Army } # Sync table replicated to the global sync table as to be copied to the user layer at sync time.
---@field Sync EngineDefinedField <{ id: string, army: Army }> # Sync table replicated to the global sync table as to be copied to the user layer at sync time.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== Unit.lua excerpt ==\n'
sed -n '130,190p' lua/sim/Unit.lua

printf '\n== Search EngineDefinedField ==\n'
rg -n "EngineDefinedField|self\.Sync\s*=\s*false|Sync\s*=" lua -g '!**/*.luac'

printf '\n== Likely type definition files ==\n'
fd -a 'EngineDefinedField' lua . || true

Repository: FAForever/fa

Length of output: 24894


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== GetSync section ==\n'
sed -n '200,250p' lua/sim/Unit.lua

printf '\n== OnDestroy section ==\n'
sed -n '2140,2195p' lua/sim/Unit.lua

printf '\n== All self.Sync usages in Unit.lua ==\n'
rg -n "self\.Sync|\.Sync\b" lua/sim/Unit.lua

printf '\n== Search for false sentinel checks around Sync ==\n'
rg -n "Sync\s*==\s*false|Sync\s*~=s*false|self\.Sync\s*=\s*false|\.Sync.*false" lua

Repository: FAForever/fa

Length of output: 3978


Include false in Unit.Sync's type. OnDestroy sets self.Sync = false, so ---@field Sync EngineDefinedField <{ id: string, army: Army } | false> matches the actual terminal state.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lua/sim/Unit.lua` at line 161, Update the `Unit.Sync` annotation in
`Unit.lua` so it matches the terminal value set by `OnDestroy`; the field is
currently typed only as the sync table, but `self.Sync` is also assigned
`false`, so expand the `---@field Sync` type to include `false` while keeping
the existing `{ id: string, army: Army }` shape.

---@field TeamSpawn 'fixed' | 'random' | 'balanced' | 'balanced_flex' | 'random_reveal' | 'balanced_reveal' | 'balanced_reveal_mirrored' | 'balanced_flex_reveal'
---@field AutoTeams EngineDefinedField <'none' | 'manual' | 'tvsb' | 'lvsr' | 'pvsi'>
---@field TeamLock EngineDefinedField <'locked' | 'unlocked'>
---@field TeamSpawn EngineDefinedField <'fixed' | 'random' | 'balanced' | 'balanced_flex' | 'random_reveal' | 'balanced_reveal' | 'balanced_reveal_mirrored' | 'balanced_flex_reveal'>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Keep TeamSpawn aligned with the actual lobby option values.

teamOptions later in this file exposes penguin_autobalance for TeamSpawn, but the new union still omits it. LuaLS will keep flagging that valid value unless the annotation is updated.

Suggested fix
----@field TeamSpawn EngineDefinedField <'fixed' | 'random' | 'balanced' | 'balanced_flex' | 'random_reveal' | 'balanced_reveal' | 'balanced_reveal_mirrored' | 'balanced_flex_reveal'>
+---@field TeamSpawn EngineDefinedField <'fixed' | 'random' | 'balanced' | 'balanced_flex' | 'random_reveal' | 'balanced_reveal' | 'balanced_reveal_mirrored' | 'balanced_flex_reveal' | 'penguin_autobalance'>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
---@field TeamSpawn EngineDefinedField <'fixed' | 'random' | 'balanced' | 'balanced_flex' | 'random_reveal' | 'balanced_reveal' | 'balanced_reveal_mirrored' | 'balanced_flex_reveal'>
---@field TeamSpawn EngineDefinedField <'fixed' | 'random' | 'balanced' | 'balanced_flex' | 'random_reveal' | 'balanced_reveal' | 'balanced_reveal_mirrored' | 'balanced_flex_reveal' | 'penguin_autobalance'>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lua/ui/lobby/lobbyOptions.lua` at line 22, The TeamSpawn annotation in
lobbyOptions.lua is missing a valid lobby option value that teamOptions already
exposes. Update the EngineDefinedField union for TeamSpawn to include
penguin_autobalance so the type matches the actual values used by teamOptions
and LuaLS stops flagging it. Keep the change scoped to the TeamSpawn field
annotation and ensure it stays aligned with the option list in this file.

Comment on lines +1 to +5
--Many fields are handled by the engine, but not nullable, so UnitBlueprints.lua will flag errors without this.



----@diagnostic disable: missing-fields

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Does lua-language-server recognize a doc-comment annotation like ----@diagnostic (four dashes) the same as ---@diagnostic (three dashes)?

💡 Result:

No, the lua-language-server does not recognize ----@diagnostic as equivalent to ---@diagnostic. The lua-language-server (LuaLS) specifically looks for LuaCATS annotations, which are defined as comments starting with exactly three dashes (---) [1][2][3]. According to the official documentation, these annotations are prefixed with ---, and they are described as being "like a Lua comment with one extra dash" [1][2]. Using four dashes (----) results in a standard Lua comment rather than a recognized annotation. Consequently, any diagnostic controls, type definitions, or other metadata provided with four dashes will be ignored by the language server [1][3]. To ensure the annotation is parsed and applied, you must use the standard three-dash ---@diagnostic syntax [3][4].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'File contents with line numbers:\n'
cat -n units/XSB1104/XSB1104_unit.bp

printf '\nSearch for diagnostic annotations in XSB1104 files:\n'
rg -n --no-heading '`@diagnostic`|LuaCATS|EmmyLua' units/XSB1104 || true

Repository: FAForever/fa

Length of output: 5698


Use three dashes for the diagnostic directiveunits/XSB1104/XSB1104_unit.bp:5 uses ----@diagnostic disable: missing-fields, which LuaLS treats as a plain comment. Change it to ---@diagnostic disable: missing-fields so the suppression applies.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@units/XSB1104/XSB1104_unit.bp` around lines 1 - 5, The diagnostic suppression
comment in XSB1104_unit.bp is using the wrong comment prefix, so LuaLS ignores
it; update the existing missing-fields directive near the top of the file to use
the standard three-dash form with the same symbol name, ensuring the suppression
is recognized by the language server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant