Skip to content

fix: don't attempt to up factions for players on wikis without factions - #8273

Merged
ElectricalBoy merged 2 commits into
mainfrom
faction-perf
Oct 23, 2026
Merged

ElectricalBoy merged 2 commits into
mainfrom
faction-perf

Conversation

@Rathoz

@Rathoz Rathoz commented Oct 22, 2026

Copy link
Copy Markdown
Member

Summary

How did you test this change?

Copilot AI balanced review requested due to automatic review settings October 22, 2026 13:40
@Rathoz
Rathoz requested review from a team as code owners October 22, 2026 13:40

Copilot AI 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.

🟢 Approval recommended

The missing-data guards address the intended behavior, with no blocking issues identified in the changed code or affected callers.

No open issues 🎉

What changed in this PR

Prevents player faction lookups on wikis without faction support and makes the shared faction module tolerate missing faction data.

Changes:

  • Adds a faction-support check before fetching player faction data.
  • Makes faction data optional and guards faction helpers when it is absent.
  • Removes the shared empty faction-data fallback.
File Description
lua/​wikis/​commons/​Player/​Ext.lua Skips player faction lookups on unsupported wikis.
lua/​wikis/​commons/​Faction/​Data.lua Removes the shared placeholder faction data.
lua/​wikis/​commons/​Faction.lua Loads faction data optionally and guards helpers with a support check.

Review effort: Balanced

@ElectricalBoy ElectricalBoy 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.

just a nitpick on missing type annotation
otherwise lgtm

Comment thread lua/wikis/commons/Faction.lua
@ElectricalBoy
ElectricalBoy merged commit 0b6d0de into main Oct 23, 2026
8 checks passed
@ElectricalBoy
ElectricalBoy deleted the faction-perf branch October 23, 2026 00:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants