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

Changed PrimitiveTypes to match GL naming. #939

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
@Harrison-Miller
Contributor

Harrison-Miller commented Aug 9, 2015

Changed the PrimitiveTypes to match GL naming.

@mantognini

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini Aug 9, 2015

Member

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.

Member

mantognini commented Aug 9, 2015

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

This comment has been minimized.

Show comment
Hide comment
@Bromeon
Member

Bromeon commented Aug 9, 2015

See also the contribution guidelines.

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Aug 9, 2015

Member

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

Member

binary1248 commented Aug 9, 2015

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

This comment has been minimized.

Show comment
Hide comment
@TankOs

TankOs Aug 10, 2015

Member

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?

Member

TankOs commented Aug 10, 2015

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

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Aug 10, 2015

Member

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.

Member

LaurentGomila commented Aug 10, 2015

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

This comment has been minimized.

Show comment
Hide comment
@TankOs

TankOs Aug 10, 2015

Member

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,
};
Member

TankOs commented Aug 10, 2015

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

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini Aug 10, 2015

Member

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. :)

Member

mantognini commented Aug 10, 2015

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

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Aug 10, 2015

Member

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.

Member

LaurentGomila commented Aug 10, 2015

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

This comment has been minimized.

Show comment
Hide comment
@TankOs

TankOs Aug 10, 2015

Member

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.

Member

TankOs commented Aug 10, 2015

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

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Aug 10, 2015

Member

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. 😉

Member

binary1248 commented Aug 10, 2015

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

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Aug 10, 2015

Member

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.

Member

LaurentGomila commented Aug 10, 2015

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 comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Aug 10, 2015

Member

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.

Member

binary1248 commented Aug 10, 2015

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

This comment has been minimized.

Show comment
Hide comment
@TankOs

TankOs Aug 10, 2015

Member

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.

Member

TankOs commented Aug 10, 2015

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

This comment has been minimized.

Show comment
Hide comment
@dabbertorres

dabbertorres Aug 10, 2015

Contributor

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!

Contributor

dabbertorres commented Aug 10, 2015

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

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini Aug 10, 2015

Member

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.

Member

mantognini commented Aug 10, 2015

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

This comment has been minimized.

Show comment
Hide comment
@Harrison-Miller

Harrison-Miller Aug 10, 2015

Contributor

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.

Contributor

Harrison-Miller commented Aug 10, 2015

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

This comment has been minimized.

Show comment
Hide comment
@TankOs

TankOs Aug 10, 2015

Member

@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?

Member

TankOs commented Aug 10, 2015

@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

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini Aug 11, 2015

Member

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.

Member

mantognini commented Aug 11, 2015

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

This comment has been minimized.

Show comment
Hide comment
@TankOs

TankOs Aug 11, 2015

Member

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.

Member

TankOs commented Aug 11, 2015

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

This comment has been minimized.

Show comment
Hide comment
@criptych

criptych Aug 11, 2015

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).

criptych commented Aug 11, 2015

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

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini Aug 12, 2015

Member

@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.)

Member

mantognini commented Aug 12, 2015

@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

This comment has been minimized.

Show comment
Hide comment
@TankOs

TankOs Aug 12, 2015

Member

hopefully politely

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

Member

TankOs commented Aug 12, 2015

hopefully politely

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

@Harrison-Miller

This comment has been minimized.

Show comment
Hide comment
@Harrison-Miller

Harrison-Miller Aug 12, 2015

Contributor

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?

Contributor

Harrison-Miller commented Aug 12, 2015

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

This comment has been minimized.

Show comment
Hide comment
@TankOs

TankOs Aug 14, 2015

Member

Thank you, looks good. 👍

Member

TankOs commented Aug 14, 2015

Thank you, looks good. 👍

@TankOs TankOs added s:accepted and removed s:undecided labels Aug 14, 2015

@TankOs TankOs added this to the 2.3.2 milestone Aug 14, 2015

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Aug 14, 2015

Member

Is this considered a bugfix or a feature?

Member

eXpl0it3r commented Aug 14, 2015

Is this considered a bugfix or a feature?

@TankOs

This comment has been minimized.

Show comment
Hide comment
@TankOs

TankOs Aug 14, 2015

Member

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

Member

TankOs commented Aug 14, 2015

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

@kimci86

This comment has been minimized.

Show comment
Hide comment
@kimci86

kimci86 Aug 18, 2015

Contributor

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

Contributor

kimci86 commented Aug 18, 2015

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

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Aug 19, 2015

Member

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

Member

eXpl0it3r commented Aug 19, 2015

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

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Aug 19, 2015

Member

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.

Member

binary1248 commented Aug 19, 2015

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.

@eXpl0it3r eXpl0it3r removed this from the 2.3.2 milestone Aug 20, 2015

@zsbzsb

This comment has been minimized.

Show comment
Hide comment
@zsbzsb

zsbzsb Nov 6, 2015

Member

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.

Member

zsbzsb commented Nov 6, 2015

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 eXpl0it3r added this to the 2.4 milestone Nov 6, 2015

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Nov 6, 2015

Member

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

Member

eXpl0it3r commented Nov 6, 2015

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

@zsbzsb

This comment has been minimized.

Show comment
Hide comment
@zsbzsb

zsbzsb Nov 6, 2015

Member

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.

Member

zsbzsb commented Nov 6, 2015

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.

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Nov 8, 2015

Member

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.

Member

Bromeon commented Nov 8, 2015

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

This comment has been minimized.

Show comment
Hide comment
@zsbzsb

zsbzsb Nov 9, 2015

Member

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. 😉

Member

zsbzsb commented Nov 9, 2015

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

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Nov 10, 2015

Member

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?

Member

eXpl0it3r commented Nov 10, 2015

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

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Nov 10, 2015

Member

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 😉

Member

Bromeon commented Nov 10, 2015

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

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Feb 22, 2016

Member

Bump.

Member

binary1248 commented Feb 22, 2016

Bump.

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Apr 25, 2016

Member

@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?

Member

eXpl0it3r commented Apr 25, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r
Member

eXpl0it3r commented May 18, 2016

@Harrison-Miller Bump 😉

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Jun 5, 2016

Member

Superseded by #1095

Member

eXpl0it3r commented Jun 5, 2016

Superseded by #1095

@eXpl0it3r eXpl0it3r closed this Jun 5, 2016

@eXpl0it3r eXpl0it3r added s:superseded and removed s:accepted labels Jun 5, 2016

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