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

Spike: Emit a host for Mono with static accessors for ScriptHost #746

Closed
wants to merge 1 commit into from

Conversation

stirno
Copy link
Contributor

@stirno stirno commented Jun 2, 2014

This is really a first-stab POC on getting custom ScriptHost impls working in Mono. Goal is to crawl the passed in ScriptHost and create a class with static proxy methods/accessors that can be used by the Mono engine.

Not perfect but it does work for PVC's ScriptHost in mono. Code needs some cleanup for sure, possibly doing something horribly wrong. Thoughts? 👍

@ztone
Copy link
Contributor

ztone commented Jun 3, 2014

I did a quick test run on this PR ... nice approach to this issue.

I would have liked some more description around this PR though, examples, next steps, what is possible, what is solves etc.

@stirno
Copy link
Contributor Author

stirno commented Jun 6, 2014

Kind of forgot about this.. Ya, I should have added more detail. I had just talked with @khellang in JabbR about it and threw it up.

Problem: Custom ScriptHost fields/methods are unavailable when running under Mono because of the MonoHost wrapper currently being used.

This solves the issue by emitting a DynamicMonoHost that has static properties/methods for the provided ScriptHost.

Need some more people to try it out but @pvcbuild is testing this right now with our admittedly simple custom ScriptHost.

@filipw
Copy link
Member

filipw commented Jun 11, 2014

very good idea @stirno, very cool spike too.
I was thinking about the same thing and the static requirement is really a difficult hurdle to clear and unfortunately emitting code might be the only way to go.

Here are my suggestions:

  • leave the original static wrapper as it's fast, the builder logic should only kick in when the host's identity is not an instance of ScriptHost
  • add tests - this looks very fragile so it definitely needs proper unit tests
  • the code should probably be changed to work against IScriptHost rather than the base class, since we only require the interface implementation
  • host can rely on private methods too which are not being emitted here. The non-backing fields should be taken into account as well

@stirno
Copy link
Contributor Author

stirno commented Jun 25, 2014

Hey guys, sorry for leaving this hanging. @khellang and I just wanted to get the conversation started on the potential solution. Agreed it needs good tests. Trying to find some time to spend on this. Hopefully this weekend.

@adamralph adamralph removed the hold label Jul 30, 2014
@glennblock
Copy link
Contributor

@stirno I am closing this as it has not had any progress in 2 years.

@glennblock glennblock closed this Jul 29, 2016
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.

5 participants