Skip to content

Re-write mono module editor code in C# #30282

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jul 5, 2019
Merged

Re-write mono module editor code in C# #30282

merged 3 commits into from
Jul 5, 2019

Conversation

@neikeq
Copy link
Contributor

@neikeq neikeq commented Jul 3, 2019

No description provided.

@neikeq neikeq added this to the 3.2 milestone Jul 3, 2019
akien-mga
akien-mga reviewed Jul 3, 2019
View reviewed changes
akien-mga
akien-mga approved these changes Jul 3, 2019
View reviewed changes
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Non-Mono changes seem good.
@neikeq
Copy link
Contributor Author

neikeq commented Jul 3, 2019

I forgot to commit one file, will fix soon.
@neikeq neikeq force-pushed the editor_in_cs_equals_win branch from 14b4184 to c49f9ee Compare July 3, 2019 16:28
aaronfranke
aaronfranke reviewed Jul 4, 2019
View reviewed changes
Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

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

Didn't get to look through everything, but here's a few things I noticed.
modules/mono/editor/GodotTools/.gitignore Outdated
Copy link
Member

@aaronfranke aaronfranke Jul 4, 2019

Choose a reason for hiding this comment

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

Should any of these be added to https://github.com/github/gitignore/blob/master/Godot.gitignore ?
Copy link
Member

@akien-mga akien-mga Jul 4, 2019

Choose a reason for hiding this comment

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

No, that repository is for project-specific .gitignore files, so it should only be for files generated in normal use of Godot.
Third party code editors are outside that scope, and github/gitignore already provides .gitignore files for them. I don't know if they provide a feature to merge several .gitignore files when you have a project using different software, but if they don't, they should :)
@neikeq neikeq force-pushed the editor_in_cs_equals_win branch 4 times, most recently from e6a1521 to 2f5c1b9 Compare July 4, 2019 19:47
@neikeq
Copy link
Contributor Author

neikeq commented Jul 4, 2019 •
edited
Loading

Alright, 9th time is the charm. CI should be happy now... ᴵ ˢʷᵉᵃʳ ᵗᵒ ᵍᵒᵈ···
@neikeq neikeq force-pushed the editor_in_cs_equals_win branch 2 times, most recently from 7d6ac9a to 588c0a3 Compare July 4, 2019 21:53
akien-mga
akien-mga reviewed Jul 5, 2019
View reviewed changes
modules/mono/mono_gd/gd_mono_utils.cpp Outdated
Copy link
Member

@akien-mga akien-mga Jul 5, 2019

Choose a reason for hiding this comment

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

Looks like Travis' mono doesn't like this: https://travis-ci.org/godotengine/godot/jobs/554433349
Copy link
Member

@akien-mga akien-mga Jul 5, 2019

Choose a reason for hiding this comment

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

Seems like we're still running 5.10.1.20 from the debian wheezy upstream repo on Travis:

Get:2 http://download.mono-project.com/repo/debian wheezy/main amd64 mono-runtime-sgen amd64 5.10.1.20-0xamarin1+debian7b1 [1,760 kB]

I don't know if there's a more recent repo compatible with Travis' Ubuntu Xenial.
Copy link
Member

@akien-mga akien-mga Jul 5, 2019

Choose a reason for hiding this comment

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

Yeah the mono source is wheezy:
https://github.com/travis-ci/apt-source-safelist/blob/0c2267bff66def573d31a8240ff7d31795e1f6fa/ubuntu.json#L418-L423

I'll see if they could add http://download.mono-project.com/repo/debian/dists/stable-xenial/ to the whitelist, but I don't have high hopes of Travis acting fast, so we should likely keep this define for now.
Copy link
Member

@akien-mga akien-mga Jul 5, 2019

Choose a reason for hiding this comment

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

Well there's my own year old issue: travis-ci/apt-source-safelist#384
Copy link
Contributor Author

@neikeq neikeq Jul 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Added a travis workaround:

is_travis = os.environ.get('TRAVIS') == 'true'

if is_travis:
    # Travis CI may have a Mono version lower than 5.12
    env_mono.Append(CPPDEFINES=['NO_PENDING_EXCEPTIONS'])
Copy link
Member

@akien-mga akien-mga Jul 5, 2019

Choose a reason for hiding this comment

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

Sounds good. So for non-Travis we would now require a version >= 5.12 I suppose?
Copy link
Contributor Author

@neikeq neikeq Jul 5, 2019

Choose a reason for hiding this comment

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

We've had that requirement for a year :P https://github.com/godotengine/godot-docs/blame/82963cf7055bad24694d88d3ecc6a4da04f14fa7/development/compiling/compiling_with_mono.rst#L11
neikeq added 3 commits July 5, 2019 09:38
@neikeq
Re-write mono module editor code in C#
270af6f 
Make the build system automatically build the C# Api assemblies to be shipped with the editor.
Make the editor, editor player and debug export templates use Api assemblies built with debug symbols.
Always run MSBuild to build the editor tools and Api assemblies when building Godot.
Several bugs fixed related to assembly hot reloading and restoring state.
Fix StringExtensions internal calls not being registered correctly, resulting in MissingMethodException.
@neikeq
Script API methods must return Ref<T> instead of Reference*
aa9908e 
ptrcall assumes methods that return a Reference type do so with Ref<T>. Returning Reference* from a method exposed to the scripting API completely breaks ptrcalls to this method (it can be quite hard to debug!).
@neikeq
Fix localize_path not always working
0639946 
We make sure the resource dir path ends with a trailing '/' for safety reasons, so we must make sure the path we compare it to does so as well.
@neikeq neikeq force-pushed the editor_in_cs_equals_win branch from 588c0a3 to 0639946 Compare July 5, 2019 07:38
@akien-mga akien-mga merged commit 6e9cb44 into godotengine:master Jul 5, 2019
@akien-mga
Copy link
Member

akien-mga commented Jul 5, 2019

Thanks!
@neikeq neikeq deleted the editor_in_cs_equals_win branch May 14, 2021 01:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement topic:dotnet

3 participants

@neikeq @akien-mga @aaronfranke