Naughty naming and float3

[zenzike]

Currently we use the convention that classes in spring are in PascalCase, prefixed by C for a class and I for an interface. For some basic types, we've even dropped that prefix.

However, this isn't what is done for float3, which is a class. I was thrown by this, since I assumed that it was a struct. We need float3 to be a class, since we are implementing compiler safe float instructions by using the streflop implementations of basic operators.

It turns out that for the AI, we'll be requiring structs with 3 floats in them, so we're actually interseted in the struct, not the class; some standardisation in the naming would have helped. This is what I suggest: rename SFloat3 to float3, and rename float3 to Float3. (Or, if people would rather we can make it CFloat3).

Any objections? If everybody is happy with this suggestion, I'll make the changes and submit a patch in a couple days.

[imbaczek]

IMHO float3 is fine, it's special anyway.

[ILMTitan]

I'm not entirely sure I understand your problem. There is no difference between structs and classes except for the the accessibility of members without declared visibility. Structs and classes can even inherit from each other. I can't understand why you would "need" float3 to be one or the other.

The only other difference is in how programmers tend to treat them, and we already treat float3 as a struct in many ways. Given that the first line of float3 is the public access specifier, float3 may as well be a struct. It would change nothing about how it is or could be used.

[Tobi]

However, this isn't what is done for float3, which is a class. I was thrown by this, since I assumed that it was a struct. We need float3 to be a class, since we are implementing compiler safe float instructions by using the streflop implementations of basic operators.

Remember struct and class are identical in C++, except that struct has default access modifier public and class has default access modifier private.

It turns out that for the AI, we'll be requiring structs with 3 floats in them, so we're actually interseted in the struct, not the class; some standardisation in the naming would have helped. This is what I suggest: rename SFloat3 to float3, and rename float3 to Float3. (Or, if people would rather we can make it CFloat3).

Types with identical name apart from uppercase/lowercase is bad for readability.

C is for concrete classes, ie. contrary to interfaces (and abstract classes). float3 is an entirely different thing: a value type, like std::string and int and float.

Therefore it really should NOT have a C prefix IMO, and actually I agree with imbaczek that float3 is fine for what it is: the name resembles it can be used similarly to e.g. float: it is a value type.

[zenzike]

Thanks; yes I know that in C++ classes and structs are the same except for their default access modifier. I should have been a little clearer in what I was saying: the AI interface requires a C type struct: a public class with no member functions, other than (if required) inline constructors with no body.

The reason we need the 3 float struct to be a C struct is that we're implementing a C interface: we can't pass classes.

I wouldn't usually recommend classes which differ in type only; the reason I did so here was the precedent set by Java with int/Integer, float/Float and others. In the sense that int or float are the PODs here, I thought it would make sense for float3 to be just the POD, ie, a struct with 3 floats, and Float3 to be the class. I still think this is a compelling argument, especially since SFloat3 is the POD here and float3 is the class, which seems a little backward.

I did expect people not to like this modification: though probably because we're used to seeing float3, and we all like to be conservative.

Good point about std::string, as with std::map, std::vector etc; basic type classes do use lower case. So it's really a moot point whether we go one way or another: I just wanted to make sure we had thought about what was going on. I do think it's pretty strange that SFloat3 is named that way though, but if nobody wants change, that's fine :)

[Tobi]

I don't even know why SFloat3 was made in the first place.

[zenzike]

I don't even know why SFloat3 was made in the first place.

It's actually the reason why it's not safe to cast float3: inheritance isn't done the same well across compilers.

[Tobi]

But SFloat3 contains the x,y,z members, so you can just cast x,y,z.

Anyway, maybe SFloat3 should just be merged into float3, as it was before? Or was there a really good reason to split it off. (TBH inheritance in value types doesn't make really much sense, not in the least because of this casting...)

[Kloot]

Auswaschbar introduced it in r5402:

* new SFloat3 with 3 float members (x,y,z), for use in unsynced code

But since float3 is still used everywhere in unsynced contexts the
real point of SFloat3 (no streflop impositions) has gotten lost a bit.

[zenzike]

I thought the inheritance is a good thing, since it means we can still have member functions in the subclass (such as synced operations), yet be able to send the base datastructure to a C interface.

If we merge float3 and SFloat3 then we can't pass the new float3 through a C interface (as I understand it) since we still have static methods defined in float3.

I'm not sure if the inline functions would also cause problems - can anybody shed light on how these are treated in the ABI?

[Auswaschbar]

My main intention to make a new base class of float3 was because it is not possible to use float3 without plenty of other objects (so I can use it in dedicated server).
And I made a base class so a synced float3 can be safely casted to an SFloat3, while it is not possible to do the other way around.

edit: I'm sorry but I couldn't think of a better name...

[hoijui]

couldnt we remove the constructor from SFloat3 and make it a struct instead of a class? then SFloat3 would be C compatible for sure.