search

bbradson / Performance-Fish

Performance Mod for RimWorld
Mozilla Public License 2.0
423 stars 34 forks source link

Apparent incompatibility with Medical Tab mod #51

Open morphine00 opened 1 month ago

morphine00 commented 1 month ago

Hi there, and thanks for Performance Fish!

I've found an apparent incompatibility with Medical Tab.

When you open the medical tab and hover over a pawn's % stat:

Foxtr0t1337 commented 2 weeks ago

This is because Medical Tab used BestThingRequest.singleDef. Performance-Fish used an UNSAFE patch, which will use that member as a placeholder for a HashSet ThingFilter.allowedDefs IN SOME RARE CASES which I can't find in vanilla. Then the HashSet SHOULD be used in ListerThings patch.

However Medical Tab will be the rare case. And it uses BestThingRequest.singleDef directly bypassing ListerThings patch so wrong type and boom. Before this unsafe patch is changed to a safe one you should disable ThingFilterPatches.BestThingRequest.

bbradson commented 2 weeks ago

@Foxtr0t1337 if you'd like to discuss implementation details of patches, especially regarding safety concerns, please DM me on discord. My username is bradson there.

The patch is designed to replace singleDef for requests that return multiple defs, where singleDef is expected to be null and direct derefencing of it would be throwing NREs without fish. These requests are intended to be used for rimworld's various listers, which store references to things and keep them organized by ThingRequest. Medical Tab is instead misusing it to access a filter's defs, and compares against null to check for validity. The correct vanilla way to access a filter's defs is to use filter.AllowedDefs or filter.AnyAllowedDef, without additional detours through requests.

Obviously fish causes the error here as it's causing singleDef to not be null where it normally would be. Workarounds such as adding another field to the request or replacing the def with a valid def that medical tab recognizes as invalid on another check can be implemented

bbradson commented 2 weeks ago

Other viable approaches are patching medical tab's use of thing requests or directly getting it fixed there. It only affects that mod after all