Author Topic: Proposal to change identifiers (Read 8547 times)

Nexus · « **on:** January 02, 2011, 05:52:23 pm »

Hello,

I think SFML 2 is a good opportunity to fix a few inconsistences regarding identifier names. I have some suggestions, even if they're not extremely crucial. Said enough, I'll just post how I personally see it

sf::Shape's methods SetOutlineWidth and GetOutlineWidth should become SetOutlineThickness and GetOutlineThickness. This is consistent with the parameter names of the static factory methods, besides the notion of thickness is clearer (since width is often used together with height).
Now that sf::Window has a WaitEvent function, it is probably of advantage to call the counterpart PollEvent instead of GetEvent. First, it's not a classical getter method; second, "poll" expresses the operating principle in a better way and explicitly shows the difference to "wait".
Maybe rename sf::RenderWindow::UseVerticalSync to EnableVerticalSync. The documentation talks about "enabling", the parameter name is "enabled", and in this way you don't have three different prefixes for setter methods in sf::RenderWindow (SetPosition, EnableKeyRepeat, UseVerticalSync).
sf::Music contains the member functions OpenFromFile and OpenFromMemory. The other resource classes sf::Font, sf::Image and sf::SoundBuffer call the same methods LoadFromFile and LoadFromMemory. In principle, both names are possible, but I wouldn't mix them.
A parameter name concern: sf::Drawable has various overloads of SetScale and Scale. Most of the parameters are "factors", but SetScale(const sf::Vector2&) takes a "scale". I suggest to distinguish better between absolute and relative scale setters, or at least to use uniform names.

Ceylo · « **Reply #1 on:** January 02, 2011, 06:43:29 pm »

I agree with you Nexus

.

Groogy · « **Reply #2 on:** January 02, 2011, 06:53:15 pm »

Some nice ideas, though why streams have Open instead of Load would be because it isn't actually loaded by the method but actually only opened the file for reading more or less.

Also, if he does go trough with it then I'll have to re-factor the ruby bindings XD Just in time for the studies to start again.

Laurent · « **Reply #3 on:** January 02, 2011, 07:02:43 pm »

I agree with everything, except the sf::Music functions.

I chose Open instead of Load on purpose, to avoid giving the feeling that once the function returns the music is loaded (the file / memory data has to exist as long as music is playing). However, maybe it's best to have consistent function names; I don't know.

panithadrum · « **Reply #4 on:** January 02, 2011, 07:08:08 pm »

I agree with you, but I don't think sf::Music should change its functions.

Music is not loaded when you play it...

Nexus · « **Reply #5 on:** January 02, 2011, 07:24:50 pm »

Ah, you are right, I didn't think about the fact that sf::Music is streaming the file part for part.

Nice to hear we all agree on the other points

Laurent · « **Reply #6 on:** January 02, 2011, 11:20:14 pm »

Quote

Now that sf::Window has a WaitEvent function, it is probably of advantage to call the counterpart PollEvent instead of GetEvent. First, it's not a classical getter method; second, "poll" expresses the operating principle in a better way and explicitly shows the difference to "wait".

Although I totally agree with this, I wonder if "poll" is common enough so that everybody can understand immediately what the function is doing. The word is used a lot in programming, but I'm not satisfied with the definition that I found in my english dictionary. It seems like it's used in a very different context and I'm afraid some beginners could be lost. What do english speaking people think about it?

By the way, I applied the other changes

Mr. X · « **Reply #7 on:** January 03, 2011, 12:07:33 am »

Nice changes!

I have another proposal: Please change sf::Image::GetWidth and sf::Image::GetHeight to sf::Image::GetSize which returns sf::Vector2i. Thats more consistent, I think.

Grimshaw · « **Reply #8 on:** January 03, 2011, 03:22:44 am »

Or just add the function in the previous post instead of replacing, could be useful : )

About the other changes, i think the API is really understandable as it is, but still, change is always good, means evolution : )

Disch · « **Reply #9 on:** January 03, 2011, 04:48:45 am »

Quote

The word is used a lot in programming, but I'm not satisfied with the definition that I found in my english dictionary. It seems like it's used in a very different context and I'm afraid some beginners could be lost. What do english speaking people think about it?

I don't think native English speakers will have problems understanding it.

But if you think it might be confusing, "PeekEvent" is another common alternative.

Terrydil · « **Reply #10 on:** January 03, 2011, 05:58:28 am »

I think PollEvent is clear enough for us English-speaking folks.

Groogy · « **Reply #11 on:** January 03, 2011, 06:19:45 am »

Darn then I have to update the binding. :-P

Anyway, Pull sound natural to me both in English and Swedish. Not that you should copy, but SDL uses Pull I think.

Groogy · « **Reply #12 on:** January 03, 2011, 07:38:46 am »

Would it be too much to ask for a list of the changes or should I just look at the revision log when I get to a computer?

Laurent · « **Reply #13 on:** January 03, 2011, 08:40:02 am »

Ok for poll, I just wanted to be sure

Peek is not so good in my opinion, in low-level libraries it often means "look at the event but keep it in the queue".

Groogy · « **Reply #14 on:** January 05, 2011, 07:23:43 pm »

Aight rbSFML got the new names now

Took me some time for that simple task, I was chosen to be lead programmer in my group at the university so I am busy all days making schedules and working on a new renderer -.-

Ooooh I wish my teachers would let me use SFML instead.