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

[0.12.5]Detours.TryDetourFromTo() doesn't work in Mac OS X (even in Linux) #54

Closed
akisute opened this issue Jan 17, 2016 · 17 comments
Closed

Comments

@akisute
Copy link

akisute commented Jan 17, 2016

I've stuck into the problem regarding to the Detours.TryDetourFromTo() when I was trying the Mod Variety Pack (https://ludeon.com/forums/index.php?topic=16735.msg188326#msg188326). Detours.TryDetourFromTo() is causing a some sort of exception in this case.

As far as I've read its implementation (https://github.com/RimWorldCCLTeam/CommunityCoreLibrary/blob/4038d86aa0cd4a252568f4c46ccde46460fffb9c/DLL_Project/StaticClasses/Detours.cs) there's no wonder why this doesn't work at all. This implementation basically relying on the implementation of the Mono x86-32 runtime, it's too unsafe and of course, it doesn't work in other runtime like x64 (all Macs run in 64 bit, you know). I understand this feature is beneficial, so at least you guys should add a strong warning about using this function like "This function doesn't work other than Windows" to notify mod developers.

@ForsakenShell
Copy link
Contributor

The code detouring is experimental right now and none of the CCL devs have a Mac to test on.

We'll try to sort this issue out and implement an internal platform specific detour to handle this but, it may take some time to sort it out.

Thanks for the report.

@akisute
Copy link
Author

akisute commented Jan 20, 2016

Thanks for reply! I know it's quite hard to support multiple platforms even with the power of Unity so it's okay. In the meantime, perhaps you may want to add some platform-specific namespaces like, for example, System.Windows (.NET) or UnityEngine.Windows (Unity) to specify the function is only available in the specific platform. I mean, detour is obviously very useful, especially while Tyran is away, you guys should keep it going, but supporting black magics like detour in multiple platforms at once might be too much.

@Karel-Kroeze
Copy link
Contributor

93bb86b attempts to fix this. We still can't test, but it didn't break the win version, so I'm updating the 0.12.5 pre-release. Feedback would be great!

@akisute
Copy link
Author

akisute commented Jan 25, 2016

Great! Thank you so much! I'm going to try this on ModVarietyPack and see what happens.

EDIT: Tested in my environment using ModVarietyPack 1.24 by replacing its Community Core Library to the latest 0.12.5.3 and... it worked! It spits out a couple of red error lines on boot but it looks like nothing critical. And actually I can play the game with it! Again thank you so much for your help!

@Karel-Kroeze
Copy link
Contributor

could you post the errros?

Also, afaik MVP only uses detours for the extra face images, do they show up in-game? Are you seeing colonists with beards, scars, etc on their face? If not, then the detour probably did not actually work.

@akisute
Copy link
Author

akisute commented Jan 25, 2016

Errors are looked to be related to the MVP itself. It says "Multiple definition files detected" or something like that. I haven't seen that error before though.

I just played the game like 15 minutes or so, so yeah... it might not be actually working. I should investigate the situation father more.

@ForsakenShell
Copy link
Contributor

We would still like confirmation one way or the other as to whether the detours work on OS X.

If it works on OS X then it works on x64 Linux. Is there any way you can confirm this (ie, MVP's beards and scars, etc)?

@simon-82
Copy link

Errors are looked to be related to the MVP itself. It says "Multiple definition files detected" or something like that. I haven't seen that error before though.

I had multiple modhelper defs. Merging them into on got rid of all these errors. Using CCL 0.12.5.3 with the detour method on custom heads works fine on pc. Can't test on Mac though.

@ForsakenShell
Copy link
Contributor

re: ModHelperDefs
Stricter checks were added to enforce the presence of a single def. Using multiple defs could cause unexpected behaviours as the sequencing can not be guaranteed at the individual mod level but is guaranteed across mods based on load order. Because of the "important nature" of what these defs are doing, using multiple defs had to be forbidden.

re: Detouring
Thanks for the confirmation that it works on Windows however, we already knew that code worked (all the CCL devs are using Windows and could test that). We still need confirmation of Mac OS X and/or 64-bit Linux (the two platforms we aren't running). Hopefully we can get this soon so we can do a new full release instead of these pre-releases.

@akisute
Copy link
Author

akisute commented Jan 29, 2016

@simon-82 can you upload the latest version of MVP that uses the pre-release version of CCL and fixed multiple defs. I'm going to try it with a new save game to confirm.

@simon-82
Copy link

@akisute Still working on the next release. The multiple defs error shouldn't matter for testing the detour thing. The modded heads are what uses the detour method. Test everything related to those. Just look at prepare carefully and see if the heads with beards / scars are available. Then make some colonists with those heads and see if they work fine in-game. If everything goes well, that should be the confirmation that it works.

@akisute
Copy link
Author

akisute commented Jan 29, 2016

@simon-82 OK! I'll do it as soon as I'm home. Thanks for details! :D

@akisute
Copy link
Author

akisute commented Jan 29, 2016

@simon-82 @ForsakenShell I've tested the pre-release version CCL CommunityCoreLibrary-0.12.5.3 with ModVarietyPack-1.24 on my Mac OS X 10.10.5 and confirmed that the detour is working as intended. Nothing suspicious found in my developer console as well. I believe you're good to ship this code! Here's screenshots:

hige1

hige2

hige3

@Karel-Kroeze
Copy link
Contributor

Awesome, all hail E! the master of C# black magic!

On Fri, 29 Jan 2016 at 13:15 akisute notifications@github.com wrote:

@simon-82 https://github.com/simon-82 @ForsakenShell
https://github.com/ForsakenShell I've tested the pre-release version
CCL CommunityCoreLibrary-0.12.5.3 with ModVarietyPack-1.24 on my Mac OS X
10.10.5 and confirmed that the detour is working as intended. Nothing
suspicious found in my developer console as well. I believe you're good to
ship this code! Here's screenshots:

[image: hige1]
https://cloud.githubusercontent.com/assets/37218/12675396/4d8fea3a-c6cd-11e5-9b06-83f57b0e7865.png

[image: hige2]
https://cloud.githubusercontent.com/assets/37218/12675399/4f2e6f10-c6cd-11e5-9571-b580131e5957.png

[image: hige3]
https://cloud.githubusercontent.com/assets/37218/12675406/53486f60-c6cd-11e5-9f5d-aa2949f61649.png


Reply to this email directly or view it on GitHub
#54 (comment)
.

@simon-82
Copy link

Good job E! 👍

@akisute
Copy link
Author

akisute commented Jan 29, 2016

You're the C# Ninja, Mr. E 😆

@ForsakenShell
Copy link
Contributor

I'm glad to see it works on 64-bit systems now. It only took a couple hours of digging to find out what the calling convention under Mac OS X and 64-bit Linux (they use the same) was.

I'll do a final code review today and if all goes well there will be a new release.

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

4 participants