Fix Tracker.AddTypeToTracker

[LozenChen]

This fixes #1020

[LozenChen]

here i make AddSpecificType can return true even if the type is abstract, aiming to fix some really edge cases like:

1. mod A has an abstract entity type E, but does not contain any non-abstract subclass in mod A.
2. mod B has some subclasses of E.
3. mod C tries to AddTypeToTracker(type: E, trackedAs: null, inherited: true) and then GetEntities<E>(), when mod A is loaded, and doesn't check if mod B is loaded.

[SnipUndercover]

Can you name a specific case of this?

[LozenChen]

https://github.com/LozenChen/TAS-Helper/blob/598d77dd7bfda322fb7cd1ffebcb9b8c10f860ee/Source/Gameplay/AutoWatchEntity/CoreLogic.cs#L24
TAS-Helper invokes AddTypeToTracker(typeof(Trigger), null, trackedAs: true)

https://github.com/DemoJameson/Celeste.SpeedrunTool/blob/6b8499e9793ce6c0cba984376c5849000982eb27/SpeedrunTool/Source/TeleportRoom/TeleportRoomUtils.cs#L502-L503
SpeedrunTool invokes GetEntitiesTrackIfNeeded(typeof(ChapterPanelTrigger))

i know Trigger is already [Tracked(true)] by vanilla. But it's bad that tracking them again leads to a different result

[LozenChen]

I just wrote a simple mod to show what happens
https://github.com/LozenChen/LozenTestMod/releases/tag/tracker

Most codes lie in https://github.com/LozenChen/LozenTestMod/blob/master/Source/Test/BugReproduce.cs

Normally

1. enable only this mod, and fully restart the game
2. console "load 7", enter room a-00
3. console "find_trigger", which just logs and does nothing else

we find that there is CameraOffsetTrigger * 1 and ChangeRespawnTrigger * 1 in this level
and that Tracker.GetEntities<AnySubclassOfTrigger>() finds no trigger.
This is as expected: CameraOffsetTrigger and ChangeRespawnTrigger themselves are not tracked

4. console "get_camera_offset_trigger", which returns tracker.GetEntitiesTrackIfNeeded<CameraOffsetTrigger>().Count

we get CameraOffsetTrigger * 1, as expected.
Step 3 can be skipped and it makes no difference

However if we

1. enable only this mod, and fully restart the game
2. before entering level, console "add_trigger_to_tracker", which calls Tracker.AddTypeToTracker(type: typeof(Trigger), trackedAs: null, inheritAll: true)

trigger itself is already [Tracked(true)], so we expect that this should have no effect

3. console "load 7", enter room a-00
4. console "find_trigger" if you like
5. console "get_camera_offset_trigger",

we get CameraOffsetTrigger * 0, unexpected.

[Wartori54]

Suggested change

	// Make sure the tracker knows about both the type and trackedAs type, to fix scenarios like `[TrackedAs(typeof(UntrackedType))]`
	bool updated = false;
	if (knownTypes is not null) {
	updated = knownTypes.Add(type);
	updated |= knownTypes.Add(trackedAsType);
	}
	// check IsAbstract later, make AddTypeToTracker(type: typeof(Celeste.CutsceneEntity), trackedAs: null, inheritAll: true) work properly
	if (type.IsAbstract) {
	return false;
	return updated;
	}
	// Make sure the tracker knows about trackedAs type, to fix scenarios like `[TrackedAs(typeof(UntrackedType))]
	// Vanilla adds the target type (`trackedAsType`) regardless of whether it's abstract or not
	bool updated = knownTypes.Add(trackedAsType);
	if (type.IsAbstract) {
	return updated;
	}

Why not just get rid of the knownTypes.Add(type)? As far as my understanding goes when a type T is in knownTypes it means that the tracker is aware of it (not just that it is on the keys of tracked) and T is tracked, tracker.GetEntities<T>() will return instances of T. (In other words: tracked[typeof(T)].Contains(typeof(T)) is true for T when T is in knownTypes.)

Furthermore, trackedAsType should always be added to knownTypes since it will be present on some list in the values (not keys) of tracked.

[LozenChen]

I agree. I reviewed the commit history again, and it looks like we were very close to a correct solution earlier

[Wartori54]

LGTM, the only edge case im currently worried about is in the case where a mod uses TrackedAs and relies on GetEntities not throwing when used on the type that had the TrackedAs applied to:

[TrackedAs(typeof(A)]
public class E : Entity { ... }

public class A : Entity { ... }

// Somewhere else
tracker.GetEntities<E>(); // This will throw now, instead of returning an empty list if `E` was also not `Tracked` manually

[Wartori54]

Please leave the comment, since it is still relevant, but move it to the right place.

[LozenChen]

If a mod uses TrackedAs and then tries GetEntities<OriginalClass>, that's already incorrect usage. I think we should just let it crash here rather than hide the bug

[Wartori54]

I agree, I'm mostly worried about backwards compat.

[JaThePlayer]

Looks correct, let's hope no mod did anything stupid (like that TrackedAs(UntrackedType) scenario from earlier)

[maddie480-bot]

The pull request was approved and entered the 3-day last-call window.
If no further reviews happen, it will end on Dec 17, 2025, 10:24 PM UTC, after which the pull request will be able to be merged.

[maddie480-bot]

The last-call window for this pull request ended. It can now be merged if no blockers were brought up.