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

Translate GDScript Code Examples to C# (Classes beginning with C) #42016

Merged
merged 1 commit into from
Sep 26, 2020

Conversation

HaSa1002
Copy link
Contributor

@HaSa1002 HaSa1002 commented Sep 12, 2020

Translates Code Examples in classes beginning with C.

Includes:

  • Callable
  • CanvasItem
  • CharFXTransform
  • Color
  • ColorRect
  • ConfigFile
  • ConfirmationDialog
  • Control
  • Crypto

Work was done while using and improving my codetranslator. I hope the ongoing effort pays out in less mistakes, altough I am not sure how much is catched.

Base PR multi-language code examples: #40613
Following: #40978

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.

I hope the ongoing effort pays out in less mistakes, altough I am sure how much is catched.

Regardless of how good your tool is, you absolutely do still need to check manually. There are a ton of problems.

doc/classes/Callable.xml Outdated Show resolved Hide resolved
doc/classes/CanvasItem.xml Outdated Show resolved Hide resolved
doc/classes/CharFXTransform.xml Outdated Show resolved Hide resolved
doc/classes/ConfigFile.xml Outdated Show resolved Hide resolved
doc/classes/Control.xml Outdated Show resolved Hide resolved
public override object GetDragData(Vector2 position)
{
object mydata = MakeData();
SetDragPreview(MakePreview(mydata));
Copy link
Member

Choose a reason for hiding this comment

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

This is also a problem with the GDScript code, but MakeData and MakePreview aren't actually defined here, they are meant as placeholders for user code. As such, this needs to be commented.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added comment

[csharp]
public override void _Ready()
{
Modulate = GetThemeColor("font_color", "Button"); //get the color defined for button fonts
Copy link
Member

Choose a reason for hiding this comment

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

As said above, GetThemeColor (and get_theme_color in GDScript) do not exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See above

doc/classes/Control.xml Outdated Show resolved Hide resolved
doc/classes/Control.xml Show resolved Hide resolved
doc/classes/Control.xml Outdated Show resolved Hide resolved
@HaSa1002
Copy link
Contributor Author

I did a second pass and adjusted my workflow for future batches, so I find more bugs in the first place. Turns out your eyes are a totally unreliable syntax checker.

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.

This PR is looking pretty good.

{
public override void _Ready()
{
SetDragForwarding(_TargetControl);
Copy link
Member

@aaronfranke aaronfranke Sep 24, 2020

Choose a reason for hiding this comment

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

If this is supposed to be public, this should be targetControl (or maybe a property TargetControl), if this is supposed to be private, this should be _targetControl and the GDScript code should be renamed to _target_control. Note that neither language example actually declares this variable, it would be nice if you did this.

Copy link
Contributor Author

@HaSa1002 HaSa1002 Sep 25, 2020

Choose a reason for hiding this comment

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

I added it as exported property, but I am not sure about the export.

Copy link
Member

Choose a reason for hiding this comment

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

You still are referencing _TargetControl instead of TargetControl on this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry. Fixed now and rebased

@HaSa1002 HaSa1002 force-pushed the docs-lang-2 branch 2 times, most recently from 9be7689 to 8011838 Compare September 26, 2020 09:46
Translates Code Examples in classes beginning with `C`.

Includes:
 * Callable
 * CanvasItem
 * CharFXTransform
 * Color
 * ColorRect
 * ConfigFile
 * ConfirmationDialog
 * Control
 * Crypto
@akien-mga akien-mga merged commit c844cc8 into godotengine:master Sep 26, 2020
@akien-mga
Copy link
Member

Thanks!

@akien-mga akien-mga added this to the 4.0 milestone Apr 29, 2021
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.

4 participants