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

Rename Reference to RefCounted #652

Closed
RandomShaper opened this issue Mar 30, 2020 · 9 comments
Closed

Rename Reference to RefCounted #652

RandomShaper opened this issue Mar 30, 2020 · 9 comments
Milestone

Comments

@RandomShaper
Copy link
Member

Describe the project you are working on:
This affects engine development rather than projects. Users can still see the name of the class, but won't matter much in projects.

Describe the problem or limitation you are having in your project:
N/A

Describe the feature / enhancement and how it helps to overcome the problem or limitation:
We have a reference-counted base class called Reference. That name is confusing, since they are not references, but things that are referred to.
The idea is to rename it to RefCounted so it describes better what it names and also to avoid hard to understand types in the C++ side, like Ref<Reference> (what?, a reference to a reference?).

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:
class Mesh : public Reference would become class Mesh : public RefCounted. Much easier to understand, right?

If this enhancement will not be used often, can it be worked around with a few lines of script?:
No.

Is there a reason why this should be core and not an add-on in the asset library?:
Not possible.

@twaritwaikar
Copy link

I think I have seen that terminology in some Lua binding library (probably LuaBridge). This means that the term RefCounted<T> is widely known outside of the engine too. But in the end, it is up to the core devs

@Xrayez
Copy link
Contributor

Xrayez commented Sep 24, 2020

I think anything will do!

RefCounted asks for completion → RefCountedObject. What about RefObject?

I imagine reduz would be in favor of a more explicit version. 😉

@neikeq
Copy link

neikeq commented Sep 26, 2020

I've also seen RefCounted in other game engines, so it's indeed pretty common.

@aaronfranke
Copy link
Member

Makes sense. The name ReferenceCounted occurred to me, I'm not necessarily in favor of it, but posting it for consideration.

@Xrayez
Copy link
Contributor

Xrayez commented Jun 4, 2021

ReferenceCounted could be a good compromise between the old and new, especially when it's more explicit. We do already have even more verbose names like RandomNumberGenerator over RNG etc.

@Xrayez
Copy link
Contributor

Xrayez commented Jun 4, 2021

I have looked at godotengine/godot#49312 and noticed this:

Ref<RefCounted> rc;

When we compare this usage against suggested ReferenceCounted:

Ref<ReferenceCounted> rc;

What looks more readable to you?

@RandomShaper
Copy link
Member Author

Judging just by that example, I'd say the second is more readable.

However, I still believe RefCounted wins overall. One reason is that 'reference' tends to be abbreviated as 'ref' in many other cases, like SafeRefCount and Ref itself.

I'd say it's better to go for RefCounted and maybe consider the univeral expansion of Ref to Reference in a separate proposal.

@Xrayez
Copy link
Contributor

Xrayez commented Jun 5, 2021

I'd say it's better to go for RefCounted and maybe consider the univeral expansion of Ref to Reference in a separate proposal.

I'm not sure whether it makes sense to discuss this in isolation, because we're talking about consistency here (even if we don't necessarily have consistency/clarity as of now). I'm mainly talking about:

The idea is to rename it to RefCounted so it describes better what it names and also to avoid hard to understand types in the C++ side, like Ref<Reference> (what?, a reference to a reference?).


That said, if there are other occurrences which make it potentially ambigious with Ref<>, then perhaps they are worth to consider renaming in the same proposal (which could've been renamed to "Resolving Ref vs Reference name inconsistencies and confusion"). But yeah, if that's what makes the review and approval process easier, so be it!

I'm also talking from the standpoint of Godot's development principles (the way I see it). I rarely see reduz copycatting names from other game engines. 😛

@akien-mga akien-mga added this to the 4.0 milestone Jun 11, 2021
@akien-mga
Copy link
Member

Implemented by godotengine/godot#49312.

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

No branches or pull requests

7 participants