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

netcore support #67

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

netcore support #67

wants to merge 1 commit into from

Conversation

shalzuth
Copy link

@shalzuth shalzuth commented Jul 8, 2021

note - this also forces all types to not be sequential. need to filter better. this pr addresses #44

@ds5678
Copy link
Contributor

ds5678 commented Jul 8, 2021

As I understand things, now that 0.4.15.3 has been released, all changes should be directed to the v0.5.x-wip branch.

One of the goals for v0.5 is generation of assemblies that are compatible with dot net 5. For example, Pass 24 has already been removed in eeaa230

You can view more information about v0.5 in the documentation folder.

Copy link
Owner

@knah knah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it is right now, it introduces regressions. 0.5.x rewrite might be the better place to introduce large changes, such as removing static constructors from enums without breaking things.

@@ -24,6 +24,7 @@ private static void GenerateStaticProxy(AssemblyRewriteContext assemblyContext,
{
var oldType = typeContext.OriginalType;
var newType = typeContext.NewType;
if (newType.IsEnum) return;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing static constructors from enum types will likely break practically everything that uses them, including boxed enums and generics with enum type parameters.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to find an example that uses a static constructor for an enum type. Do you happen to have one? I tried a generic method (i.e. from https://stackoverflow.com/questions/79126/create-generic-method-constraining-t-to-an-enum), but can't get mono working with it yet.

@@ -149,8 +150,8 @@ private static void EmitLoadTypeNameString(this ILProcessor ctorBuilder, Assembl
ctorBuilder.Emit(OpCodes.Ldstr, originalTypeReference.FullName);
else
{
ctorBuilder.Emit(newTypeReference.IsByReference ? OpCodes.Ldc_I4_1 : OpCodes.Ldc_I4_0);
ctorBuilder.Emit(OpCodes.Call, imports.Module.ImportReference(new GenericInstanceMethod(imports.Il2CppRenderTypeNameGeneric) {GenericArguments = {newTypeReference}}));
ctorBuilder.Emit(newTypeReference.IsByReference ? OpCodes.Ldc_I4_1 : OpCodes.Ldc_I4_0);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be only used for methods that somehow don't get a token (in fact, this should have been removed a long time ago as in practice everything has tokens). It would be better to actually investigate those cases when there are tokenless methods and fix those.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the case of:
IL2CPP.RenderTypeName<someType*>(true) - .net doesn’t like class pointers as a generic type input.

@shalzuth
Copy link
Author

shalzuth commented Jul 8, 2021

I’ll look at 0.5. Will keep this open in the meantime for discussion if that’s okay.

@shalzuth shalzuth marked this pull request as draft July 8, 2021 17:06
@shalzuth
Copy link
Author

shalzuth commented Jul 8, 2021

is also invalid .net (where mono doesn't care). I think we can remove it without any issue, and this looks like a bug anyway. If any method in an injected class returns a non-void value, it would return here instead of actually returning the result.

@shalzuth
Copy link
Author

shalzuth commented Jul 8, 2021

My GC was free'ing the TypeToClassDelegate from

ourOriginalTypeToClassMethod = Detour.Detour(targetMethod, new TypeToClassDelegate(ClassFromTypePatch));
as well. This delegate should be alloc'd, or made static, correct? Doing so fixes my implementation. The other delegate hookedClassFromName is static, so it doesn't get GC'd ever.

Edit - nvm, fixed here

private static readonly List<object> PinnedDelegates = new List<object>();

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

Successfully merging this pull request may close these issues.

None yet

3 participants