Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apparent incompatibility with Medical Tab mod #51

Open
morphine00 opened this issue May 23, 2024 · 3 comments
Open

Apparent incompatibility with Medical Tab mod #51

morphine00 opened this issue May 23, 2024 · 3 comments

Comments

@morphine00
Copy link

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:

  • You can't see the pop-up with info about which upgrades are affecting the stat
  • Left- or right-click to see the available medical operations.
@Foxtr0t1337
Copy link
Contributor

Foxtr0t1337 commented Jun 19, 2024

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
Copy link
Owner

@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
Copy link
Owner

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

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

No branches or pull requests

3 participants