[SceneTree] Annotation Interferes with Discards

aroelke commented 4 months ago

Since C# 7, it's been possible to use the _ character to discard unused values, but adding the [SceneTree] annotation to a class defines it as a field used to access the scene hierarchy and prevents its use as a discard. This causes something like this (or the examples given in Microsoft's documentation for the discard) to fail to compile:

foreach ((_, var key) in someDictionary)
{
    ...
}

It doesn't seem to interfere with its usage in switch expressions, but the above definitely does fail. Is this intentional?

Cat-Lips commented 4 months ago

Hi aroelke.

Unfortunately, it was a minor oversight when the project was created whilst searching for a minimal impact yet notable & compilable character. As an integral part of the package, it was a bit late to change it once discovered. Of course, the obvious workaround is to not use discards when using [SceneTree], but it would be nice if it could be fixed it without impacting consumers. Otherwise, we might just have to leave this one as a known issue :(

aroelke commented 4 months ago

Oh, okay. Is the concern that an IDE might try to automatically update and force users to refactor? If not, couldn't you just increment the major version number to indicate a backwards-incompatible change, like https://semver.org/ specifies and Godot itself did between versions 3 and 4? Or is there other reason I'm missing?

Cat-Lips commented 4 months ago

Yes, changing it would require everyone to make the change, but a search and replace for '_.' should suffice. We could look at making the change for version 3, but we'd want to check with downstream consumers first and maybe vote on it. If we want to go ahead with this, what character options would you like/suggest as a replacement?

Another workaround is to use uniquely named nodes or use a property to access the scene tree to avoid underscore interference.

(In fact, given uniquely named nodes is the preferred way of doing things in Godot 4 (and the other workarounds), I'd be inclined to not make the change unless strongly requested by the community.)

aroelke commented 4 months ago

You could replace it with something like Nodes or Children, or SceneTree to match the annotation if that's allowed, just to make it explicit what it's referring to. Or it could look like a function, like GetNodes() maybe, to match Godot's built-in GetChildren() function. Alternately, you could do away with _. entirely and just access child nodes directly just like you currently do with unique ones, but then you wouldn't be able to name a unique node with the same name as a non-unique node, which apparently is allowed.

The problem with the interference is that it happens if you annotate a class with [SceneTree] whether or not you actually use _.ChildNode to access a child node. Otherwise that workaround would be a pretty good solution to the problem.

Cat-Lips commented 4 months ago

The problem with the interference is that it happens if you annotate a class with [SceneTree] whether or not you actually use _.ChildNode to access a child node.

Ah, ok. That I didn't realise.

Thanks for making me aware of this. I'll see what I can do.

Cat-Lips commented 4 months ago

Another workaround is to add var:

  //foreach ((_, var value) in dic)
  foreach (var (_, value) in dic)

  //_ = DiscardReturn();
  var _ = DiscardReturn();

Not ideal, but...

benjiwolff commented 3 weeks ago

There is already an alternative field _sceneTree. We could update the readme to mention it instead of _.

On top of that we could mark _ as obsolete and provide a quick-fix to replace it with _sceneTree.

At some point, _ can be removed.