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

Add DebugView for Array and Dictionary, based of the DebugView from the .NET Foundation #90060

Merged
merged 1 commit into from
Apr 8, 2024

Conversation

warquys
Copy link
Contributor

@warquys warquys commented Mar 31, 2024

I am working on reimplementing the debug view for the C# runtime. Initially, I was creating a new debug view. Then, I incorporate the debug view from the .NET Foundation. This approach ensures consistency between the C# runtime’s enumerable and the GODOT enumerable.

This is why the comment of MIT license of the .net Foundation.

Copy link
Member

@raulsntos raulsntos left a comment

Choose a reason for hiding this comment

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

This looks good to me, thanks.

Not sure what to do about the MIT license header. cc @akien-mga

@raulsntos raulsntos modified the milestones: 4.x, 4.3 Apr 5, 2024
@akien-mga
Copy link
Member

akien-mga commented Apr 5, 2024

What's the actual source for this DebugView.cs file? The versions I found on Google all only have "All rights reserved." but seem to be older versions.

To be fair, the code is so simple that I'm not even sure this falls under copyright, it's literally just declaring 3 simple classes to fit an interface, and APIs are not copyrightable. Dotnet code includes these two lines of license statement in all files for consistency (like we do), but in this case there isn't much to cover.

C# is verbose but technically it's barely 30 lines of actual code, most of which are actually specific to Godot (using our own implementations of CopyTo for Godot's Array and Dictionary, and KeyValuePair). There's just a few attributes which make it a DebugView implementation, which are described in the docs https://learn.microsoft.com/en-us/dotnet/framework/debug-trace-profile/enhancing-debugging-with-the-debugger-display-attributes and can likely be considered public domain (either way they're part of an interface, not implementation code).

So all in all I don't think that file needs to be encumbered with a specific license/copyright statement, it could be considered Godot code.

@akien-mga akien-mga changed the title Add DebugView for Array and Dictionary, based of the DebugView from the .NET fondation Add DebugView for Array and Dictionary, based of the DebugView from the .NET Foundation Apr 5, 2024
@warquys
Copy link
Contributor Author

warquys commented Apr 5, 2024

The source of this debug view is the .net runtime, debug view is :
ICollectionDebugView.cs, IDictionaryDebugView.cs and DebugViewDictionaryItem.cs.

@warquys warquys force-pushed the CSharp-DebugInfo branch from 217ab8b to f8c482b Compare April 5, 2024 17:55
@warquys
Copy link
Contributor Author

warquys commented Apr 5, 2024

I force pushed to divide the commit in two and change the indentation.
In this way if it is considered that the license does not apply I would just remove the last commit.

@akien-mga
Copy link
Member

Thanks for the links!

I think this is trivial enough to not require attribution, so I think we can drop the second commit.

@warquys warquys force-pushed the CSharp-DebugInfo branch from d7fb9a3 to f8c482b Compare April 7, 2024 15:55
@warquys
Copy link
Contributor Author

warquys commented Apr 7, 2024

Ok, i drop it

@akien-mga akien-mga merged commit 368d6db into godotengine:master Apr 8, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

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.

C# Debugger display and type proxy
4 participants