Changed PrimitiveTypes to match GL naming.

[Harrison-Miller]

Changed the PrimitiveTypes to match GL naming.

[mantognini]

As you can see, this changes the public API and therefore breaks existing code. And that cannot be done for SFML 2.x.

If you have some suggestions that include breaking changes, come over to our forum to discuss them for SFML 3.

PS: next time, please at least include a description with your PR.

[Bromeon]

See also the contribution guidelines.

[binary1248]

A non-breaking way of "fixing" this issue would be to introduce the new names while keeping the old ones and marking them deprecated.

[TankOs]

I've also sometimes wondered about the names of some primitive types, so I generally agree to this PR. And @binary1248's idea of deprecating the old names sounds very good indeed, this would allow for inclusion before SFML 3.

Did we really have to close this?

[LaurentGomila]

So you want to break the public API, add deprecated symbols and some confusion, just for a few 's'?

Let's keep this closed and move on to something important.

[TankOs]

I disagree, I find this to be important. It's part of the naming consistency, which should be the same in every spot in SFML. Otherwise you could also argue that typos are not important, e.g. "steColor": Why break the public API just for a "e" and "t"?

The public API would not be broken. The wrong symbol names would be kept until the next major version, next to the correct ones:

enum PrimitiveType
{
  LinesStrip = x,
  LineStrip = x,
};

[mantognini]

I really dislike the idea of having duplicate enum values, from both the user and implementation POV: It's just a huge open door for bugs and confusion.

We can do it for 3.0 without any issue, if we really want to. Let's not clutter things just for an "extra" s just now, please. :)

[LaurentGomila]

Why is this considered an issue in the first place? Nobody ever said that we have to name things 1:1 compared to OpenGL. If that was a rule, I would have named these enums accordingly from the start. And if it's a new rule, then I'm not aware of it 😛

If the current enums are grammatically incorrect then ok, that will be an argument.

[TankOs]

We can do it for 3.0 without any issue, if we really want to. Let's not clutter things just for an "extra" s just now, please. :)

I'm fine with that. If we agree on it, we can simply flag this for 3.0.0, rather than closing it, though.

Nobody ever said that we have to name things 1:1 compared to OpenGL

Of course not, but the names in the other big graphics framework are identical: https://msdn.microsoft.com/en/library/windows/desktop/Bb205124(v=VS.85).aspx

So my question is rather: Why choose a set a of names when the whole world is using different ones? It's not too dramatic, but this is probably what will raise confusion, not changing the names to the correct ones.

[binary1248]

It's not that we follow anybody else's naming, it's just that the current names are grammatically incorrect. You can read any literature on computer graphics that is not related to programming in any way and they will also use the same naming. In fact, that is the reason why all the other libraries ended up with the same names, they based them on literature and not each other.

Also, like Tank said, adding these names now while keeping the old ones doesn't break anything, which is why I suggested it in the first place. And I also don't see how this can introduce bugs that actually successfully compile. It's not always necessary to be that paranoid about such trivial things. 😉

[LaurentGomila]

the current names are grammatically incorrect

The discussion should have started with this statement. Now we can discuss seriously about what to do.

I would be fine with deprecating them now, but since it's a very minor "issue", and people don't seem to complain too much about it (this is the very first time, the enums have been there for years), I see no reason to rush and add some clutter to the public API now, we can also wait for SFML 3.

[binary1248]

This is the second part to making a library appealing to more "serious" developers. If we just make a clean cut when SFML 3 starts, people with financial investment in old code and therefore higher risk will be put off or will never migrate because it costs too much or is too risky. SFML 3 won't come out overnight or even in the next months. It is a long term goal. If we introduce the new names (among other things) now, newer code bases could be built on top of them already and "be ready" for SFML 3. Forcing developers to wait for SFML 3 to be released and rush to adapt their legacy code just to use some of the new features is a disservice to long-time SFML users.

Look at any other "serious" library. They almost never make a clean cut even when releasing major versions. It is a death sentence to the user-base. Deprecation exists for a reason. If it didn't make sense, libraries would not make use of it. The more "serious" the libraries' user-base is, the more necessary having a migration strategy becomes.

[TankOs]

I'm not sure if it's grammatically incorrect (I see it similar to "item list" vs "items list", but I'm also no native speaker), but what I don't understand is why it's a valid discussion now. If grammatically correct or not: There's no single reason to break with the world's standards, even if just for "some s's".

If it's something for 3.x or 2.x can be discussed. Personally I'd be happy if this wouldn't just be closed.

One project that's doing a good job on updates and deprecation is Django. Features can be flagged as being deprecated in the docs, which will also emit warnings when being used. As every sane developer will read the FULL upgrade announcement anyway, it's a very nice way to stay up-to-date with your code. It gives the people enough time to make the switch and get used to the new stuff.

Indeed, waiting until 3.0 with all of the API-breaking things, which can make use of deprecation, doesn't make sense to me.

[dabbertorres]

Now, I don't have much experience in maintaining large libraries, but, for what it's worth, would starting a 3.0 branch be possible/worthwhile?

Don't make it an "official" release branch or whatnot, just for everything that can't be in 2.x, start to merge it all into a 3.0 branch. To get a head start essentially. If there are enough desired features that can't be implemented in 2.x, I don't see much point in waiting, especially if you can keep them separate. Call it a "pre-alpha" or something, make it known that it may have API breaking changes so you can modify as much as needed.
Especially since 2.x is mostly bug-fixing at this point. There are a lot of feature/* branches at the moment. Combine them so people can start testing them. This would be analogous to other projects that have dev/nightly branches, alongside the master/stable branch.

Plus, it'd let @binary1248 start seeing progress on the features he wants to add.

people don't seem to complain too much about it

Well, SFML is used by a lot of beginners and people that haven't used OpenGL directly. They most likely wouldn't know about the differing names. That'd be my guess as to why no one has brought this up before.

Anyways, just my 2 cents as a SFML user!

[mantognini]

There are a few things mixed up here so to somewhat sum it up:

• A. Do we want to rename things?
• B. If yes, when do we do it?

Regarding A, I guess we should also look for other strings in the code; there might be more. So if there's a majority that feels it's a good thing (regardless of when it's done) an issue should be open instead of this PR.

Now, every library use some slightly different vocabulary so saying that X, Y and Z use term T therefore we should do the same feels is not a valid argument: We also need to make sure the arguments behind their choice is valid. From that point of view, the only valid argument I see is the grammar and since I'm no native speaker I leave it those people to determine the truth. But it's not enough to say "some use T so should we".

As to B. when, for something that small I really feel it should be delayed until 3.0 because of a few things. The most important one is maintenance: If we do it now we need to add a synonym and therefore double the switch cases in our own code. But there are also some impact on the users: imagine you're using a library that interact with SFML that's up-to-date and doesn't use deprecated feature; however in your code you still use a resource file that use the old string and when you deserialize it you don't notice the small difference and your app now crashes because the library doesn't support both, or anything else is broken. Just imagine what happens when the minor problems that are generated by having to synonyms can do when combined.

Yes, this is a very unlikely scenario but still, we have no good reason to take that risk especially since we're not able to predict everything.

Also, the cost of upgrading from SFML 2.x to 3.x will not be significantly increased just by this change. And honestly I would not value deprecation: Just look how wrong the Java API got with all its deprecated feature. Adding a deprecated warning is IMO not a good strategy at all.

So, to summarize my opinion: A. whatever you like, B. not now.

[Harrison-Miller]

On the point of A, let me clarify why I thought it necessary to change at some point. Myself along with a few other irc users noticed the extra 's' and were confused. Our initial thought was that it might have been some kind of translation error. However given that these names were meant to represent the underlying GL primitive types, I waved translation issues as not possible. The extra 's' did not seem intentional to myself, but instead a typo. It is not only that it is grammatically incorrect ("Are you a triangles fan?" asked Fred. Chris responded, "no I like the circles better, their batting average is superb!"), but that it just seems like a typo in the naming in general. This is just what I, a user of sfml saw it as. Whether or not it is implemented in 2.x; I don't care.

[TankOs]

@mantognini, I think in this case "others use T, so should we" is very valid, because: a) OpenGL, which we abstract, uses the terms. b) D3D, which we don't abstract, but which is another huge graphics API, uses them. c) Everybody in the world uses them. d) As it points out, it's even grammatically wrong.

So, if there's a well-known and defined term, I see no reason keeping an alternative. It was a mistake by the author, no big deal, but let's not avoid fixing mistakes because of breaking APIs.

How does Java do it?

[mantognini]

Notice I was not arguing against the change in itself but on the argumentation for the change. Now, as pointed by a few people the current version has a typo, there's no point in arguing for/against it because of this grammar rule. (Without it, it would be a mindless reflexion, like many happened in human history.)

Regarding Java, basically you have a huge standard library (nothing like STL) cluttered with deprecated features (a lot of function here and there, entire classes/modules too) which makes it really a pain to use: you don't know where to find the right tool among a deprecated army of similar looking functionalities (okay, I'm probably exaggerating a bit but you got the point). Why it happened to be like that is the real interesting question. For the case of Java they never decided to remove stuff from their standard lib, unlike what's going to happen in C++17 for a handful features. They never really went from version 2 to version 3 without introducing breaking change (I'm not talking about the language itself here). And that's the fear I get when people speak about deprecating features: will we get stuck with SFML 2.15 instead of making a jump?

Another aspect of deprecating too much things (where's the limit?) is that it gives a feeling that the library was not well thought initially and is quite unstable actually. So, at least for me, it's a sign that it might not be good enough for decent applications.

[TankOs]

And that's the fear I get when people speak about deprecating features: will we get stuck with SFML 2.15 instead of making a jump?

No, the whole reason for marking things as being deprecated is that they will be removed in the (near) future, most likely with major version bumps, because API compatibility will be broken anyway.

Another aspect of deprecating too much things (where's the limit?) is that it gives a feeling that the library was not well thought initially and is quite unstable actually. So, at least for me, it's a sign that it might not be good enough for decent applications.

For me: No limit. What needs to be fixed/improved, should be fixed/improved. In my opinion it's impossible to write perfect code from the beginning (it's even never possible). Mistakes will be made, better approaches invented etc.

So in contrary to your impression, I think that libraries that change (regularly/often) to improve, are worth to be used, because it's ensured that all involved people are trying to get the best out of it, instead of just staying with the "old" solutions because they work and changing stuff is too sophisticated.

Example: I'd never do smart pointers everywhere in SFGUI again. If I had the time left, I'd rewrite it nearly from scratch now. That would for sure annoy some users of it, but in order to improve, things sometimes have to change for the better.

[criptych]

If we do it now we need to add a synonym and therefore double the switch cases in our own code.
But there are also some impact on the users: imagine you're using a library that interact with
SFML that's up-to-date and doesn't use deprecated feature; however in your code you still use
a resource file that use the old string and when you deserialize it you don't notice the small
difference and your app now crashes because the library doesn't support both, or anything else
is broken.

Since it's an enum, as long as the "synonyms" have the same numerical value, the library won't care. Their names exist only in the header, not exported like functions.

For the case of Java they never decided to remove stuff from their standard lib

I think that's related to the distribution model. Java supports many apps with a single runtime library, so they need to keep all the old classes around for legacy software written against an older library. Otherwise, you'd need multiple versions of Java installed (like with Python 2.x vs 3.x).

[mantognini]

@TankOs I understand your will to improve things and I agree that we have to make things better (who would disagree with that :P) but we have slightly different strategies toward that goal. Nevertheless ->

@criptych
that's an interesting point you got there about the distribution model. And probably because of its huge size, it was not the best thing to make a comparison between SFML and Java. In any case ->

-> if we step back and look at the discussion, it seems there's only one bloke (me) that actively (& hopefully politely) disagree with this immediate strategy so you guys should probably go on a devise a patch/open an issue/... ;)

(I'm on my phone now so it's a bit hard to find it again but I know we discussed how to technically implement a deprecation on enum -- I just don't remember the conclusion.)

[TankOs]

hopefully politely

Sure :) Regarding deprecated features, I'll open a discussion in our internal dark dungeon. ;)

[Harrison-Miller]

I've made a patch that has the suggested duplicates for deprecated names. I am however a little worried that some compilers may not correctly number the not explicitly numbered enums. Thoughts? Should all the the values be explicitly defined to be safe?

[TankOs]

Thank you, looks good. 👍

[eXpl0it3r]

Is this considered a bugfix or a feature?

[TankOs]

A bugture! Bugfix if you asked me, as the original spelling is wrong.

[kimci86]

I think the deprecated names need documentation comments with \deprecated command.

[eXpl0it3r]

What do others think about bug vs feature and inclusion into 2.3.2 or not?

[binary1248]

This is an unintended mistake thus a bug, fixing it doesn't add or remove anything to the feature set of SFML thus it can't be classed as a feature.

We can't add this to 2.3.2 because strictly speaking this changes the external API although I don't think it would break the API or ABI in any way.

[zsbzsb]

Just looking through the issues and noticed this had no milestone... will this be included in 2.4 since there is no code breakage? It would also be a good change to apply the new deprecated macro.

[eXpl0it3r]

Looks like the milestone didn't get set.
How exactly can the deprecated macro be applied here?

[zsbzsb]

How exactly can the deprecated macro be applied here?

Ah right, I forgot that we the macro can't be applied to enums. Either way the now deprecated names still should have documentation stating that they are deprecated. A single comment above them won't make it into the docs.

[zsbzsb]

Shouldn't the "= XXX" be spaced out so it all lines up?

[Bromeon]

Are there no other occurrences of these enumerators in the SFML source code, including examples?
Furthermore, we should not forget to adapt the official bindings and tutorials.

[zsbzsb]

Furthermore, we should not forget to adapt the official bindings

No worries, CSFML/SFML.Net will get updated accordingly before they get the 2.4 tag. 😉

[eXpl0it3r]

Are there no other occurrences of these enumerators in the SFML source code, including examples?

You could have just started a search. 😜

Example in the documentation in VertexArray.hpp needs adjustment.
Example in the documentation in Vertex.hpp needs adjustment.
OpenGL ES code in RenderTarget.cpp (and here as well) needs adjustment.

@Harrison-Miller Can you update this PR?

[Bromeon]

You could have just started a search. 😜

I know, but I didn't have the time to do so when I wrote the comment. Since the pull requester is likely to modify the code anyway, he could just do a search for occurrences in the IDE and directly adapt them 😉

[binary1248]

Bump.

[eXpl0it3r]

@Harrison-Miller It's still missing some of the adaptions I mentioned, also it would be nice to get a single commit. Do you have time to get around to it?

[eXpl0it3r]

@Harrison-Miller Bump 😉

[eXpl0it3r]

Superseded by #1095