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

Replace most usages of IsDynamicCodeCompiled with IsDynamicCodeSupported #5914

Merged
merged 3 commits into from
Jul 12, 2023

Conversation

frenzibyte
Copy link
Member

IsDynamicCodeSupported returns true in iOS when building with interpreter mode on, so it's probably better to use that instead (kind of as a way of testing dynamic code on iOS). Note that I have kept the TransformCustom cases as they generate CIL instructions, which I'm not sure would outperform reflection in an interpretation environment (I have tested this and I didn't see any noticeable differences, but felt safer not to change it).

@smoogipoo
Copy link
Contributor

Have you tested that this runs too?

@peppy
Copy link
Member

peppy commented Jul 10, 2023

Let's save this for after the next framework release, as to not add any potential regressions.

@frenzibyte
Copy link
Member Author

Have you tested that this runs too?

Yeah I've tested this just now on iOS, not sure about other platforms like Android though.

Let's save this for after the next framework release, as to not add any potential regressions.

Yeah for sure.

@bdach
Copy link
Collaborator

bdach commented Jul 10, 2023

not sure about other platforms like Android though

You know what, I tested and am not sure either. Especially so that the OP is so wonderfully vague about why this is being changed that it matches the actual xmldocs of both methods in uselessness.

Anyways, in testing, this seems to change something on android:

image

Is this good or bad? Who knows. Game runs, at least. Music plays, tracks loop, so maybe fine.

I'll also bring up that this was already mentioned for whatever reason over half a year ago. Why? No idea.

@frenzibyte
Copy link
Member Author

What you have there shows that the game is running using interpretation.

@smoogipoo smoogipoo enabled auto-merge July 12, 2023 12:30
@smoogipoo smoogipoo merged commit e1a603f into ppy:master Jul 12, 2023
@frenzibyte frenzibyte deleted the fix-bool-usage branch July 12, 2023 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants