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

Feat: Embedded link.xml in assembly #194

Merged
merged 9 commits into from
May 25, 2021
Merged

Conversation

bitsandfoxes
Copy link
Contributor

@bitsandfoxes bitsandfoxes commented May 21, 2021

Instead of having to copy the link.xml into Assets/ we can embed the file into the assembly.

https://sentry.io/organizations/sentry-sdks/issues/2410989926/?project=5439417&query=is%3Aunresolved

@bitsandfoxes bitsandfoxes added the Feature New feature or request label May 21, 2021
@bitsandfoxes bitsandfoxes changed the title Feat: Embedded link.xml Feat: Embedded link.xml in assembly May 21, 2021
[Test]
public void LinkXml_Embedded()
{
var resourceNames = Assembly.GetAssembly(typeof(SentryUnity)).GetManifestResourceNames();
Copy link
Contributor Author

@bitsandfoxes bitsandfoxes May 21, 2021

Choose a reason for hiding this comment

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

I'm not sure if this is the proper way to get to the assembly so any feedback would be greatly appreciated.

@bruno-garcia
Copy link
Member

Does Unity support this? link.xml is used by Unity at compile time. It doesn't need to ship with the game, but IL2CPP needs this file when it runs to make sure it doesn't drop the types defined in it.

@bitsandfoxes
Copy link
Contributor Author

Yes they do! https://forum.unity.com/threads/the-current-state-of-link-xml-in-packages.995848/ <- last reply point a)
I tested it on an IL2CPP build on the phone and added the link to the captured event to the description.

@bruno-garcia
Copy link
Member

@bitsandfoxes if you remove link.xml completely, does it fail to capture?
I'm unsure at this point which rules we really need.

Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

This is a great find btw! Much nicer way to manage the link.xml

test/Sentry.Unity.Tests/EmbeddedResourcesTests.cs Outdated Show resolved Hide resolved
test/Sentry.Unity.Tests/EmbeddedResourcesTests.cs Outdated Show resolved Hide resolved
@bitsandfoxes
Copy link
Contributor Author

bitsandfoxes commented May 25, 2021

@bitsandfoxes if you remove link.xml completely, does it fail to capture?
I'm unsure at this point which rules we really need.

It does not even compile if the file is missing. You'd have to remove it from the csproj and then it doesn't show up on the manifest and then the test fails.

@bitsandfoxes bitsandfoxes merged commit 3c69984 into main May 25, 2021
@bitsandfoxes bitsandfoxes deleted the feat/embedded-link.xml branch May 25, 2021 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants