Skip to content

Conversation

@wlsnmrk
Copy link
Contributor

@wlsnmrk wlsnmrk commented Apr 25, 2023

C#'s EditorInspectorPlugin::_CanHandle() method takes an argument of type GodotObject, but the tutorial page for inspector plugins gave an override example taking type Variant.

Additionally, the same page had one C# line that was causing horizontal scroll in the code panel, and I've included a fix for that.

wlsnmrk added 2 commits April 25, 2023 14:06
Updated Inspector Plugin tutorial to reflect that C#'s
EditorInspectorPlugin::_CanHandle() takes a GodotObject, not a
Variant.
Put a line-break and indentation to avoid horizontal scrolling in
the displayed code.
@wlsnmrk wlsnmrk marked this pull request as draft April 25, 2023 18:39
@wlsnmrk
Copy link
Contributor Author

wlsnmrk commented Apr 25, 2023

Converting to draft as I've found some additional issues with the code on this page:

  • _ParseProperty() signature is also incorrect
  • There's an undeclared variable used in _ParseProperty()
  • UpdateProperty() is not exposed as an override-able method for EditorProperty should be _UpdateProperty()

wlsnmrk added 2 commits April 25, 2023 15:11
Changed example code for EditorInspectorPlugin::_ParseProperty():
* Fixed method signature
* Use new enum type of parameter for comparison instead of int.
* Use "name" parameter as property-identifying argument to
AddPropertyEditor() instead of undeclared variable
Updated example to use virtual method _UpdateProperty() instead of
non-virtual method UpdateProperty().
@wlsnmrk wlsnmrk changed the title Fix parameter type in C# for inspector-plugin tutorial Fix C# issues in inspector-plugin tutorial Apr 25, 2023
@wlsnmrk wlsnmrk marked this pull request as ready for review April 25, 2023 19:22
@wlsnmrk
Copy link
Contributor Author

wlsnmrk commented Apr 25, 2023

Should be good to go now, with example code on that page compiling/working.

@YuriSizov YuriSizov added bug enhancement area:manual Issues and PRs related to the Manual/Tutorials section of the documentation labels Apr 26, 2023
@YuriSizov YuriSizov requested a review from raulsntos April 26, 2023 08:18
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.

The changes are correct, at some point the API changed before the 4.0 release and I forgot to update this documentation.

Comment on lines +152 to +154
public override bool _ParseProperty(GodotObject @object, Variant.Type type,
string name, PropertyHint hintType, string hintString,
PropertyUsageFlags usageFlags, bool wide)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we usually split long method signature lines in the documentation, but I think it's fine to start doing it. I'm not sure what should be the maximum character length though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No worries either way - I was asked to break lines that were causing horizontal scroll (in both C# and GDScript) on an earlier PR, so I assumed that was the preferred approach.

The C# style guide suggests 100 characters, but I had to break this at around 90 to avoid scroll.

@mhilbrunner mhilbrunner merged commit 872e5ff into godotengine:master Apr 26, 2023
@mhilbrunner
Copy link
Member

Thanks! And thanks for the review. Merged!

@wlsnmrk wlsnmrk deleted the inspector-plugin-csharp-fixes branch April 26, 2023 17:05
@mhilbrunner
Copy link
Member

Pushed to 4.0 and stable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:manual Issues and PRs related to the Manual/Tutorials section of the documentation bug enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants