C#: Replace StringNameCache with SNAME

[raulsntos]

StringNameCache was a struct that has existed before SNAME that seems to have the same purpose, now it's no longer needed.

[AThousandShips]

This is not equivalent but uses the same syntax as SceneStringNames, unsure if there's any performance loss but this stores a single copy whereas SNAME has one copy per location, this also allows renaming the use easily in one place if needed

[raulsntos]

this stores a single copy whereas SNAME has one copy per location

You are right, the only one that seems to be used in multiple places is script/source (3). Not sure if it's worth keeping just for that one.

this also allows renaming the use easily in one place if needed

Renaming them would break compatibility since these are names of methods exposed in Object so I don't expect them to change.

[AThousandShips]

_property_get_revert is also used twice

[KoBeWi]

The performance is the same, there's only the extra memory cost (in this case very minimal).
SceneStringNames can be preferred if available, but we don't really use it consistently, e.g. searching for SNAME("changed") gives 70 results.

[bitsawer]

You should probably remove the actual CACHED_STRING_NAME() macro define too as it is now unused.

godot/modules/mono/csharp_script.cpp

Line 71 in 713bfaf

#define CACHED_STRING_NAME(m_var) (CSharpLanguage::get_singleton()->get_string_names().m_var)

[AThousandShips]

Not only is it unused it doesn't work any more

[akien-mga]

For the record, GDScript seems to have a similar pattern:

GDScriptLanguage::GDScriptLanguage() {
	calls = 0;
	ERR_FAIL_COND(singleton);
	singleton = this;
	strings._init = StaticCString::create("_init");
	strings._static_init = StaticCString::create("_static_init");
	strings._notification = StaticCString::create("_notification");
	strings._set = StaticCString::create("_set");
	strings._get = StaticCString::create("_get");
	strings._get_property_list = StaticCString::create("_get_property_list");
	strings._property_can_revert = StaticCString::create("_property_can_revert");
	strings._property_get_revert = StaticCString::create("_property_get_revert");
	strings._script_source = StaticCString::create("script/source");

[raulsntos]

For the record, GDScript seems to have a similar pattern

I'm not sure, in GDScript's case they may use them more often. Are you saying we should keep using the same pattern in the .NET module?

StringNameCache used to have more members, but they were removed in 513ee85 and e235cef so I had the impression this struct was on its way out.

As I understand it, the only benefit of using this struct over SNAME is when the name is used in multiple places because SNAME will create one instance per location whereas this struct will only create a single instance for each name. That would be useful if we were using these names in a lot of places, but the only one we use in multiple places is script/source (and even this one is only used 3 times, so it didn't seem worth keeping the struct around just for that one).

_property_get_revert is also used twice

I can only find one usage, maybe you got it mixed up with _property_can_revert.

[akien-mga]

Seems good to me.

[akien-mga]

Thanks!