Implement fallback for ghost structure rendering#3331
Implement fallback for ghost structure rendering#3331calyythae wants to merge 1 commit intoopenfrontio:mainfrom
Conversation
Fallback for ghost structure rendering if WebGL fails wrapped the init into try catch, if it fails then it logs the warning hin the console and continues
WalkthroughIntroduces fallback rendering flow for the StructureIconsLayer when WebGL is unavailable. Adds error handling during renderer initialization, stores the ghost unit type, and enables building without visual ghost structures when rendering fails, addressing Firefox WebGL issues. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/client/graphics/layers/StructureIconsLayer.ts`:
- Around line 404-409: The fallback path in StructureIconsLayer (inside the
block that calls buildWithoutGhost) skips the upgrade-vs-build check; change
buildWithoutGhost to be async, call actions() to obtain buildableUnits inside
it, and replicate the same logic used in the normal flow (the canUpgrade check
and fallthrough canBuild check found in the code that emits
SendUpgradeStructureIntentEvent or BuildUnitIntentEvent) so that when WebGL is
unavailable you emit SendUpgradeStructureIntentEvent for upgradeable targets and
only emit BuildUnitIntentEvent when canUpgrade is false but canBuild is true;
ensure you reference the same event constructors and unitType checks used in the
existing flow so behavior matches exactly.
| if (!this.ghostUnit) { | ||
| if (this.fallbackGhostType !== null) { | ||
| this.buildWithoutGhost(e, this.fallbackGhostType); | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the file structure and line count
wc -l src/client/graphics/layers/StructureIconsLayer.tsRepository: openfrontio/OpenFrontIO
Length of output: 120
🏁 Script executed:
# Read the specific code sections mentioned: 404-409, 422-442, 446-463
sed -n '400,470p' src/client/graphics/layers/StructureIconsLayer.tsRepository: openfrontio/OpenFrontIO
Length of output: 2335
🏁 Script executed:
# Find buildWithoutGhost method definition and buildableUnits usage
rg -n 'buildWithoutGhost|buildableUnits' src/client/graphics/layers/StructureIconsLayer.ts -A 3Repository: openfrontio/OpenFrontIO
Length of output: 587
🏁 Script executed:
# Search for fallbackGhostType assignments and where it's used
rg -n 'fallbackGhostType' src/client/graphics/layers/StructureIconsLayer.ts -B 2 -A 2Repository: openfrontio/OpenFrontIO
Length of output: 856
🏁 Script executed:
# Search for where ghostUnit is cleared or becomes null
rg -n 'ghostUnit\s*=' src/client/graphics/layers/StructureIconsLayer.ts -B 1 -A 1Repository: openfrontio/OpenFrontIO
Length of output: 231
🏁 Script executed:
# Look for actions() method and buildableUnits definition
rg -n 'buildableUnits|actions\(' src/client/graphics/layers/StructureIconsLayer.ts -B 2 -A 2Repository: openfrontio/OpenFrontIO
Length of output: 432
🏁 Script executed:
# Find the GameAPI or related definition to understand buildableUnits structure
rg -n 'buildableUnits' src/core -A 2 | head -50Repository: openfrontio/OpenFrontIO
Length of output: 925
🏁 Script executed:
# Look at BuildableUnit definition
rg -n 'interface BuildableUnit|type BuildableUnit' src/core -A 5 | head -30Repository: openfrontio/OpenFrontIO
Length of output: 437
🏁 Script executed:
# Check the actions() method signature more thoroughly
rg -n 'actions\(' src/core/game -B 2 -A 8 | head -60Repository: openfrontio/OpenFrontIO
Length of output: 634
🏁 Script executed:
# See how the normal flow uses actions() - line 302-320 context
sed -n '295,330p' src/client/graphics/layers/StructureIconsLayer.tsRepository: openfrontio/OpenFrontIO
Length of output: 1290
🏁 Script executed:
# Check how ghostUnit.buildableUnit is populated - trace back to see where it comes from
rg -n 'ghostUnit\s*=' src/client/graphics/layers/StructureIconsLayer.ts -B 5 -A 10Repository: openfrontio/OpenFrontIO
Length of output: 1028
🏁 Script executed:
# See the createGhost method to understand how buildableUnit is initially set
sed -n '470,530p' src/client/graphics/layers/StructureIconsLayer.tsRepository: openfrontio/OpenFrontIO
Length of output: 1882
🏁 Script executed:
# Check if there's any special handling for fallback mode or no-WebGL scenarios
rg -n 'renderLayerFallback|fallback' src/client/graphics/layers/StructureIconsLayer.ts -B 3 -A 3Repository: openfrontio/OpenFrontIO
Length of output: 1252
🏁 Script executed:
# Check what calls createStructure and how fallback differs from normal flow
rg -n 'createStructure|renderLayer' src/client/graphics/layers/StructureIconsLayer.ts | head -20Repository: openfrontio/OpenFrontIO
Length of output: 314
🏁 Script executed:
# Verify if actions() is called anywhere else in the fallback context
rg -n 'actions\(' src/client/graphics/layers/StructureIconsLayer.ts -B 5 -A 5Repository: openfrontio/OpenFrontIO
Length of output: 414
🏁 Script executed:
# Let me trace how the backend handles BuildUnitIntentEvent vs SendUpgradeStructureIntentEvent
# to understand the actual impact of the difference
rg -n 'BuildUnitIntentEvent|SendUpgradeStructureIntentEvent' src/core -lRepository: openfrontio/OpenFrontIO
Length of output: 49
🏁 Script executed:
# Check if there are tests or documentation about fallback mode behavior
find . -name "*.test.ts" -o -name "*.spec.ts" | xargs rg -l "StructureIconsLayer|fallback" 2>/dev/null | head -5Repository: openfrontio/OpenFrontIO
Length of output: 89
🏁 Script executed:
# Verify the learning from the retrieved memory about buildableUnits and upgrade behavior
sed -n '1030,1050p' src/core/game/PlayerImpl.tsRepository: openfrontio/OpenFrontIO
Length of output: 762
🏁 Script executed:
# Check how BuildUnitIntentEvent is handled - does it validate on backend?
rg -n 'class BuildUnitIntentEvent|handle.*BuildUnitIntent' src -A 5 | head -40Repository: openfrontio/OpenFrontIO
Length of output: 427
🏁 Script executed:
# Check SendUpgradeStructureIntentEvent handling
rg -n 'class SendUpgradeStructureIntentEvent|handle.*SendUpgradeStructureIntent' src -A 5 | head -40Repository: openfrontio/OpenFrontIO
Length of output: 396
🏁 Script executed:
# Check the execution classes to understand the backend validation
rg -n 'BuildUnitIntentEvent|SendUpgradeStructureIntentEvent' src/core/execution -B 2 -A 2 | head -60Repository: openfrontio/OpenFrontIO
Length of output: 49
🏁 Script executed:
# Search for where BuildUnitIntentEvent is processed in the backend
rg -n 'BuildUnitIntent' src/core -lRepository: openfrontio/OpenFrontIO
Length of output: 86
🏁 Script executed:
# Search for upgrade execution specifically
rg -n 'UpgradeStructure|upgradeUnit' src/core/execution -B 2 -A 5 | head -80Repository: openfrontio/OpenFrontIO
Length of output: 6591
🏁 Script executed:
# Check GameRunner to see how intents are routed
rg -n 'BuildUnitIntentEvent|SendUpgradeStructureIntentEvent' src/core/GameRunner.ts -B 3 -A 3Repository: openfrontio/OpenFrontIO
Length of output: 49
🏁 Script executed:
# Check how the backend actually processes BuildUnitIntentEvent in GameRunner or Schemas
rg -n 'case.*BuildUnitIntent|BuildUnitIntent.*:' src/core -A 5Repository: openfrontio/OpenFrontIO
Length of output: 49
🏁 Script executed:
# Check if there's validation of canBuild when executing BuildUnitIntentEvent
rg -n 'canBuild|canUpgrade' src/core/execution/PlayerExecution.ts -B 2 -A 2 | head -50Repository: openfrontio/OpenFrontIO
Length of output: 49
🏁 Script executed:
# Look at actual BuildUnit execution
rg -n 'buildUnit|BuildUnit' src/core/execution/PlayerExecution.ts -B 3 -A 5Repository: openfrontio/OpenFrontIO
Length of output: 49
🏁 Script executed:
# Search for where game events are processed - likely in GameRunner
rg -n 'case BuildUnitIntent|case SendUpgradeStructure|switch.*intent' src/core -A 3 | head -60Repository: openfrontio/OpenFrontIO
Length of output: 345
🏁 Script executed:
# Look for intent handling in GameRunner
rg -n 'intent' src/core/GameRunner.ts -B 2 -A 3 | head -80Repository: openfrontio/OpenFrontIO
Length of output: 49
🏁 Script executed:
# Check Schemas to see if validation happens there
rg -n 'BuildUnitIntent|SendUpgradeStructureIntent' src/core/Schemas.ts -B 2 -A 5 | head -80Repository: openfrontio/OpenFrontIO
Length of output: 1188
🏁 Script executed:
# Find ExecutionManager switch statement that handles BuildUnitIntent
sed -n '50,150p' src/core/execution/ExecutionManager.tsRepository: openfrontio/OpenFrontIO
Length of output: 3261
🏁 Script executed:
# Search for where BuildUnitIntent and UpgradeStructureIntent are handled
rg -n 'build_unit|upgrade_structure' src/core/execution/ExecutionManager.ts -B 2 -A 8Repository: openfrontio/OpenFrontIO
Length of output: 864
🏁 Script executed:
# Check ConstructionExecution to see if it validates canBuild
sed -n '1,60p' src/core/execution/ConstructionExecution.tsRepository: openfrontio/OpenFrontIO
Length of output: 2214
🏁 Script executed:
# Check if ConstructionExecution does validation
rg -n 'canBuild|canUpgrade|isValidCoord' src/core/execution/ConstructionExecution.ts -B 2 -A 2Repository: openfrontio/OpenFrontIO
Length of output: 323
🏁 Script executed:
# Check more of ConstructionExecution to see full validation logic
sed -n '58,80p' src/core/execution/ConstructionExecution.tsRepository: openfrontio/OpenFrontIO
Length of output: 725
🏁 Script executed:
# Verify how canBuild and canUpgrade work in PlayerImpl
rg -n 'public canBuild|public canUpgrade' src/core/game/PlayerImpl.ts -A 3Repository: openfrontio/OpenFrontIO
Length of output: 197
🏁 Script executed:
# Check findUnitToUpgrade which determines upgrade behavior
rg -n 'findUnitToUpgrade' src/core/game/PlayerImpl.ts -B 2 -A 10Repository: openfrontio/OpenFrontIO
Length of output: 1054
🏁 Script executed:
# Also check the canUpgrade flow in buildableUnits
sed -n '1036,1080p' src/core/game/PlayerImpl.tsRepository: openfrontio/OpenFrontIO
Length of output: 1477
Fallback build path skips upgrade check, risking build instead of upgrade in no-WebGL mode.
The normal flow (lines 412-442) checks canUpgrade and conditionally emits either SendUpgradeStructureIntentEvent or BuildUnitIntentEvent. The fallback flow (lines 446-467) always emits BuildUnitIntentEvent, bypassing the upgrade check entirely. When WebGL rendering fails, this causes upgradeable structures to send build intents instead of upgrade intents. Backend validation (canBuild) prevents invalid coordinates but doesn't enforce the upgrade-vs-build distinction, so the upgrade action is lost.
Make buildWithoutGhost async, fetch buildableUnits via actions(), and apply the same canUpgrade/canBuild logic as the normal flow.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/client/graphics/layers/StructureIconsLayer.ts` around lines 404 - 409,
The fallback path in StructureIconsLayer (inside the block that calls
buildWithoutGhost) skips the upgrade-vs-build check; change buildWithoutGhost to
be async, call actions() to obtain buildableUnits inside it, and replicate the
same logic used in the normal flow (the canUpgrade check and fallthrough
canBuild check found in the code that emits SendUpgradeStructureIntentEvent or
BuildUnitIntentEvent) so that when WebGL is unavailable you emit
SendUpgradeStructureIntentEvent for upgradeable targets and only emit
BuildUnitIntentEvent when canUpgrade is false but canBuild is true; ensure you
reference the same event constructors and unitType checks used in the existing
flow so behavior matches exactly.
github-project-automation
bot
moved this from Triage
to Development
in OpenFront Release Management
|
@calyythae I would advice against having this PR merged. There is an Issue for the underlying problem: #2147. Recently a fix has been proposed which will be in a PR soon hopefully. So that will fix the actual root issue instead of working around it. |
2 participants
If this PR fixes an issue, link it below. If not, delete these two lines.
Resolves #3322
Description:
Fallback for ghost structure rendering if WebGL fails
wrapped the init into try catch, if it fails then it logs the warning hin the console and continues
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
canonulldreinull_