Skip to content

Refactor ThingsById system to use non static data#626

Merged
notfood merged 2 commits intorwmt:devfrom
Tick-git:Refactor-ThingsById-System
Aug 17, 2025
Merged

Refactor ThingsById system to use non static data#626
notfood merged 2 commits intorwmt:devfrom
Tick-git:Refactor-ThingsById-System

Conversation

@Tick-git
Copy link
Copy Markdown
Contributor

@Tick-git Tick-git commented Aug 17, 2025

Label: 1.6, 1.5, Fix

The error in this PR probably didn’t occur often before, but with gravship it will happen more frequently since maps are generated and abandoned often. I also encountered this in #581, but couldn’t reproduce it at the time.

Issue:

The static data causes problems when two saves are played in one process.

public static Dictionary<int, Thing> thingsById = new();

In the first game, many maps are used, therefore some static cached things have a high mapIndex.

In the next game, fewer maps are used and one map is abandoned.

The static data from save before is not cleared correctly, and when map gets abandoned:

public class ThingsById

 public void UnregisterAllFrom(Map map)
 {
     thingsById.RemoveAll(kv => kv.Value.Map == map);
 }

calls a map getter that uses Find.Maps and tries to access it with the thing high mapIndex from the old save, which leads to an out-of-bounds error.

Fix

The PR removes the static data and introduces a non static ThingById class where the instance is part of the multiplayer game instance. This ensures we get a clean data structure with every new load. It also prevents direct access to the data structure by introducing wrapper methods for improved maintainability.

The static data causes problems when two saves are played in one process.

In the first game, many maps are used, so some things have a high mapIndex.

In the next game, fewer maps are used and one map is abandoned.

When the static data is not cleared correctly, and when map gets abandoned,

thingsById.RemoveAll(kv => kv.Value.Map == map);

calls a map getter that uses Find.Maps and tries to access it with the thing mapIndex from the old save, which leads to an out-of-bounds error.
@notfood notfood added fix Fixes for a bug or desync. 1.5 Fixes or bugs relating to 1.5 (Not Anomaly). 1.6 Fixes or bugs relating to 1.6 (Not Odyssey). labels Aug 17, 2025
@notfood notfood moved this to In review in 1.6 and Odyssey Aug 17, 2025
@notfood notfood merged commit 9228e27 into rwmt:dev Aug 17, 2025
1 check passed
@github-project-automation github-project-automation bot moved this from In review to Done in 1.6 and Odyssey Aug 17, 2025
@notfood notfood moved this to Backlog in Backports Aug 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.5 Fixes or bugs relating to 1.5 (Not Anomaly). 1.6 Fixes or bugs relating to 1.6 (Not Odyssey). fix Fixes for a bug or desync.

Projects

Status: Done
Status: Backlog

Development

Successfully merging this pull request may close these issues.

2 participants