Skip to content

bugfix: Add missing kindof to RailroadGuideAIUpdate - #3360

Open
DrGoldFish1 wants to merge 1 commit into
TheSuperHackers:mainfrom
DrGoldFish1:add-missing-kindoff-trains
Open

DrGoldFish1 wants to merge 1 commit into
TheSuperHackers:mainfrom
DrGoldFish1:add-missing-kindoff-trains

Conversation

@DrGoldFish1

@DrGoldFish1 DrGoldFish1 commented Sep 25, 2026 •

Copy link
Copy Markdown

This Pr fixes an issue that allows trains to drive through supplycenters, fakebuildings and internetcenters.
Now they will all get destroyed.

Also this is my first actual fix.

2026-09-25.17-04-07.mp4
2026-09-25.17-02-12.mp4

Fixes #2317

@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 7e35a046-7df3-44d4-9f90-490b4d013ba0

📥 Commits

Reviewing files that changed from the base of the PR and between a95e04d and 9f4dfa0.

📒 Files selected for processing (2)
  • Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/RailroadGuideAIUpdate.cpp
  • GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/RailroadGuideAIUpdate.cpp

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


Walkthrough

Railroad collision handling now treats faction structures as hostile when RETAIL_COMPATIBLE_CRC is not defined. After structure-specific handling, structures do not continue to unit-physics collision handling.

Changes

Railroad collision handling

Layer / File(s) Summary
Structure checks and collision response
Generals/.../RailroadGuideAIUpdate.cpp, GeneralsMD/.../RailroadGuideAIUpdate.cpp
When RETAIL_COMPATIBLE_CRC is not defined, the hostile-structure check includes faction structures and retains the existing kind checks. Structure collisions return after structure-specific handling, including when the structure is not killed by the hostile check or a ready demo trap.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: xezon

Merge Risk: ⚪ Minimal · up to 9f4df

The structure checks cover the relevant kinds in each game variant. No identified issue remains that should delay merging.

Architecture Summary

Architecture risk: 🔵 Low · up to 9f4df

The change affects 2 systems.

Changed systems: Generals, GeneralsMD

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — Generals (service) was modified; 1 changed file maps to changed impact.
  • observed — GeneralsMD (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/RailroadGuideAIUpdate.cpp: The hostile-structure condition adds isFactionStructure() when RETAIL_COMPATIBLE_CRC is not defined; the existing explicit kind checks remain. After the existing hostile impact-sound and kill handling, the new return prevents non-hostile structures from continuing into physics-based collision handling.
  • observed — Modified behavior in Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/RailroadGuideAIUpdate.cpp: Adds a return after structure-specific handling, so structures that do not match the hostile condition no longer proceed to the later collision logic.
  • observed — Modified behavior in GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/RailroadGuideAIUpdate.cpp: The hostile-structure condition retains the existing explicit kind and rebuild-hole checks and adds isFactionStructure() when RETAIL_COMPATIBLE_CRC is disabled.
  • observed — Modified behavior in GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/RailroadGuideAIUpdate.cpp: onCollide now returns after handling any structure, including structures that are not killed by the hostile-structure check or a ready demo trap; those structures no longer proceed to unit-physics collision handling.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Title check ✅ Passed The title clearly identifies the RailroadGuideAIUpdate bug fix and matches the reported collision behavior change.
Description check ✅ Passed The description explains that trains will destroy supply centers, fake buildings, and internet centers, which matches the changeset objectives.

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

Comment on lines +260 to +265
#if RETAIL_COMPATIBLE_CRC
||
other->isKindOf( KINDOF_FS_INTERNET_CENTER ) ||
other->isKindOf( KINDOF_FS_SUPPLY_CENTER ) ||
other->isKindOf( KINDOF_FS_FAKE )
#endif

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Medium AIUpdate/RailroadGuideAIUpdate.cpp:260

Non-retail-compatible builds still let trains pass through KINDOF_FS_INTERNET_CENTER, KINDOF_FS_SUPPLY_CENTER, and KINDOF_FS_FAKE structures because these checks are compiled out when RETAIL_COMPATIBLE_CRC == 0. Move the checks outside the conditional compilation guard so the collision fix applies to all supported builds.

-#if RETAIL_COMPATIBLE_CRC
-#endif
Also found in 1 other location(s)

Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/RailroadGuideAIUpdate.cpp:257

The new KINDOF_FS_INTERNET_CENTER, KINDOF_FS_SUPPLY_CENTER, and KINDOF_FS_FAKE checks are entirely inside #if RETAIL_COMPATIBLE_CRC. Any supported non-retail-compatible build with RETAIL_COMPATIBLE_CRC == 0 compiles them out, so trains still pass through these structures and the fix does not apply in that build.

🤖 Copy this AI Prompt to have your agent fix this:
In file @GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/RailroadGuideAIUpdate.cpp around lines 260-265:

Non-retail-compatible builds still let trains pass through `KINDOF_FS_INTERNET_CENTER`, `KINDOF_FS_SUPPLY_CENTER`, and `KINDOF_FS_FAKE` structures because these checks are compiled out when `RETAIL_COMPATIBLE_CRC == 0`. Move the checks outside the conditional compilation guard so the collision fix applies to all supported builds.

Also found in 1 other location(s):
- Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/RailroadGuideAIUpdate.cpp:257 -- The new `KINDOF_FS_INTERNET_CENTER`, `KINDOF_FS_SUPPLY_CENTER`, and `KINDOF_FS_FAKE` checks are entirely inside `#if RETAIL_COMPATIBLE_CRC`. Any supported non-retail-compatible build with `RETAIL_COMPATIBLE_CRC == 0` compiles them out, so trains still pass through these structures and the fix does not apply in that build.

@greptile-apps

greptile-apps Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

[Medium risk] Adds a conditional check to railroad collision logic.

The PR is not ready to merge because the Generals collision fix regresses in non-retail-compatible builds.

Findings

  1. P1 Building kinds no longer destroyed ▶
Summary

The PR changes railroad collisions in both game variants to use a faction-structure check and to return after handling structures.

  • The Generals helper does not recognize the three building kinds whose explicit checks were removed.
  • The GeneralsMD helper does recognize them.

Reviews (3) · Last reviewed commit: "bugfix: Add missing kindof to RailroadGu..."

@DrGoldFish1
DrGoldFish1 force-pushed the add-missing-kindoff-trains branch from 103d8f1 to a95e04d Compare September 25, 2026 15:11
||
other->isKindOf( KINDOF_FS_INTERNET_CENTER ) ||
other->isKindOf( KINDOF_FS_SUPPLY_CENTER ) ||
other->isKindOf( KINDOF_FS_FAKE )

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

How about other->isFactionStructure instead of more mumbo jumbos?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Made the suggested changes, I also tested these changes and seem to work like intended. Civilian buildings like the tunnel and train station are not effected and all faction buildings die.

@DrGoldFish1
DrGoldFish1 force-pushed the add-missing-kindoff-trains branch from a95e04d to 9f4dfa0 Compare September 25, 2026 22:19
other->isKindOf(KINDOF_FS_TECHNOLOGY) ||
other->isKindOf(KINDOF_REBUILD_HOLE)
#if !RETAIL_COMPATIBLE_CRC
|| other->isFactionStructure()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Building kinds no longer destroyed

In a non-retail-compatible Generals build, this replacement stops trains from destroying Internet Centers, Supply Centers, and fake buildings. Here, isFactionStructure() checks only the four kinds already listed above it, so it cannot replace the three explicit checks that were removed. When a train hits one of those buildings, the collision handler now returns without destroying it.

Suggested change
|| other->isFactionStructure()
|| other->isKindOf(KINDOF_FS_INTERNET_CENTER) ||
other->isKindOf(KINDOF_FS_SUPPLY_CENTER) ||
other->isKindOf(KINDOF_FS_FAKE)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

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.

Train Does not destroy all types of faction buildings when colliding

2 participants