Skip to content

Commit ab5b044

Browse files
authored
Fix train was destroyed message spam (openfrontio#2774)
## Description: Trains are made of a primary unit (`TrainExecution.train`) followed by 6 cars (`TrainExecution.cars`). Currently when any of the units is destroyed, a message "Your Train was destroyed" is shown. In worst case scenario, when all cars are blasted by a nuke, 7 messages are displayed to the user, but the train continues. Since the actual logic is unaffected as long as the primary unit stays alive, displaying messages is confusing. This PR fixes it. The message is only displayed when the primary unit is destroyed. The following cars are only there for fancy visuals. ## Screencast ##### Current - multiple messages for single train https://github.com/user-attachments/assets/3df04c71-d899-4f68-af83-36c9d0ffa730 First nuke was a test. Second nuke destroyed the entire train, prompting 7 messages. Third nuke destroyed one full train and then some. Prompting many too many messages. ##### Fixed - one message, only if train was fully destroyed https://github.com/user-attachments/assets/1f3840a7-6c62-487d-af3a-82de39dad9e8 First train was only partially destroyed, no message was shown, delivery mission completed A+. Following 2 trains had the front engine destroyed and therefore were promptly decommissioned. ## Please complete the following: - [x] I have added screenshots for all UI updates - [ ] I process any text displayed to the user through translateText() and I've added it to the en.json file - [ ] I have added relevant tests to the test directory - [x] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced ## Please put your Discord username so you can be contacted if a bug or regression is found: moleole
1 parent ae6293f commit ab5b044

4 files changed

Lines changed: 37 additions & 14 deletions

File tree

src/client/graphics/SpriteLoader.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,11 +80,18 @@ export const loadAllSprites = async (): Promise<void> => {
8080
* The train sprites rely on the train attributes and not only on its type
8181
*/
8282
function trainTypeToSpriteType(unit: UnitView): TrainTypeSprite {
83-
return unit.trainType() === TrainType.Engine
84-
? TrainTypeSprite.Engine
85-
: unit.isLoaded()
86-
? TrainTypeSprite.LoadedCarriage
87-
: TrainTypeSprite.Carriage;
83+
const trainType = unit.trainType();
84+
85+
switch (trainType) {
86+
case TrainType.Engine:
87+
case TrainType.TailEngine:
88+
return TrainTypeSprite.Engine;
89+
case TrainType.Carriage:
90+
default:
91+
return unit.isLoaded()
92+
? TrainTypeSprite.LoadedCarriage
93+
: TrainTypeSprite.Carriage;
94+
}
8895
}
8996

9097
const getSpriteForUnit = (unit: UnitView): ImageBitmap | null => {

src/core/execution/TrainExecution.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ import { TrainStation } from "../game/TrainStation";
1414
export class TrainExecution implements Execution {
1515
private active = true;
1616
private mg: Game | null = null;
17-
private train: Unit | null = null;
18-
private cars: Unit[] = [];
17+
private train: Unit | null = null; // primary unit
18+
private cars: Unit[] = []; // stored back to front
1919
private hasCargo: boolean = false;
2020
private currentTile: number = 0;
2121
private spacing = 2;
@@ -69,6 +69,7 @@ export class TrainExecution implements Execution {
6969
if (this.train === null) {
7070
throw new Error("Not initialized");
7171
}
72+
7273
if (!this.train.isActive() || !this.activeSourceOrDestination()) {
7374
this.deleteTrain();
7475
return;
@@ -113,7 +114,7 @@ export class TrainExecution implements Execution {
113114
this.cars.push(
114115
this.player.buildUnit(UnitType.Train, tile, {
115116
targetUnit: this.destination.unit,
116-
trainType: TrainType.Engine,
117+
trainType: TrainType.TailEngine,
117118
}),
118119
);
119120
for (let i = 0; i < this.numCars; i++) {

src/core/game/Game.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,7 @@ export enum UnitType {
217217

218218
export enum TrainType {
219219
Engine = "Engine",
220+
TailEngine = "TailEngine",
220221
Carriage = "Carriage",
221222
}
222223

src/core/game/UnitImpl.ts

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -265,13 +265,11 @@ export class UnitImpl implements Unit {
265265
this._active = false;
266266
this.mg.addUpdate(this.toUpdate());
267267
this.mg.removeUnit(this);
268-
if (displayMessage !== false && this._type !== UnitType.MIRVWarhead) {
269-
this.mg.displayMessage(
270-
`Your ${this._type} was destroyed`,
271-
MessageType.UNIT_DESTROYED,
272-
this.owner().id(),
273-
);
268+
269+
if (displayMessage !== false) {
270+
this.displayMessageOnDeleted();
274271
}
272+
275273
if (destroyer !== undefined) {
276274
switch (this._type) {
277275
case UnitType.TransportShip:
@@ -296,6 +294,22 @@ export class UnitImpl implements Unit {
296294
}
297295
}
298296

297+
private displayMessageOnDeleted(): void {
298+
if (this._type === UnitType.MIRVWarhead) {
299+
return;
300+
}
301+
302+
if (this._type === UnitType.Train && this._trainType !== TrainType.Engine) {
303+
return;
304+
}
305+
306+
this.mg.displayMessage(
307+
`Your ${this._type} was destroyed`,
308+
MessageType.UNIT_DESTROYED,
309+
this.owner().id(),
310+
);
311+
}
312+
299313
isActive(): boolean {
300314
return this._active;
301315
}

0 commit comments

Comments
 (0)