Krozark / SFML-book

SFML Blueprint book, source code
BSD 2-Clause "Simplified" License
73 stars 27 forks source link

Is this a joke? #2

Closed zsbzsb closed 9 years ago

zsbzsb commented 9 years ago

Do you even test your code or care about coding standards?

'and' does not compile in C++

ok, I stand corrected on the 'and' keyword. But for reference, it doesn't work in some compilers without including the header and 99.99% of C++ code doesn't use that.

leading double underscores is forbidden in C++ (reserved by compiler implementations)

why do you have commented code all over the place?

for all words this is beyond belief, you copy a C++ union with memcpy! memcpy is meant for raw bytes, not for defined classes/unions/structs that have clearly defined copy ctors. not to mention the fact of how ironic it is because you do it from your own copy ctor. not to mention the fact that the memory may not be contigues.

http://stackoverflow.com/a/12135856

Your git submodule idea completely failed, the idea is to not include the source directly in your own code base. And yea, copying entire libs like SFGUI is not the way to handle dependencies.

This isn't even all that is wrong with the code and your design (I haven't mentioned any code structure things yet)...

last but not least, unless you wrote these games yourself, you stole the ship.png asset from other games (I can't find it in any open art websites)

and its not last or least, most of your art assets from the 2d iso game are shamelessly stolen too (unless you own this repo). Yes it is MIT, but doing that it is nice to reference the source of the assets because as it stands it appears you made them yourself. Lets not forget the clause from MIT...

"The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software."

I didn't bother checking the rest of the assets, but I can probably assume its the same situation.

Are you trying to write proper, clean C++ (which is what SFML aims at), and teachable code or just trying to get a book with your name on the cover? Writing bad C++ code and publishing it is a way to get your book on the do not read list.

Krozark commented 9 years ago

First of All, have you see that there is no release version for the moment? there is maybe a reason for that.

And is a valid keyword : http://en.cppreference.com/w/cpp/keyword/and leading underscore : I've delete them (except if I've forget something) Please .... look at the sf::Event implementation before saying that memcopy is not a valid way to copy it (explained in my book)

Commented code will be removed (in the next day), and the code will be cleaned to to have the same coding convention everywhere.

Your git submodule idea completely failed, the idea is to not include the source directly in your own code base. And yea, copying entire libs like SFGUI is not the way to handle dependencies

I know that, but it's simpler for beginner because they don't have to download them one by one. In my book, I explain how to install (properly) all the dependencies. By the way it allow user to use the download from zip button, without having to do

git clone --recursive ...

I haven't stole the ship, I've find it under creative common 0, so .... where is the trouble? I've forget to add a link to the original creator of some assert, and some of theme are may by a friend of me for this book, so please, stop saying that I've stole everything.

Are you trying to write proper, clean C++ (which is what SFML aims at), and teachable code or just trying to get a book with your name on the cover? Writing bad C++ code and publishing it is a way to get your book on the do not read list.

First, I haven't say "I will write a book to have my name on it" NO. The publisher contact me, and offer me this opportunity. Secondly, I wrote some proper C++ code, with sometime some C like code for optimization (explained in the book again).

Finally, what is wrong with you? If you don't like this project, just skip it, that's it. I usually appreciate feedbacks but your isn't constructive at all.

Krozark commented 9 years ago

But for reference, it doesn't work in some compilers without including the header and 99.99% of C++ code doesn't use that

I know that visual studio don't support it, unless you specify a flag (I don't remember it). And this in not the only think that MSVC don't support from the standard. Clang and Gcc don't have any trouble with this.

Moreover, is it a valid reason to think like that : "because it's not use by people, don't use it" ?

zsbzsb commented 9 years ago

And is a valid keyword : http://en.cppreference.com/w/cpp/keyword/and

I updated my first post before your first reply, point still is some compilers don't implement it properly without including the header or flag (I wasn't aware there was a flag). MSVC to name one is broken in this regard (and if you are aiming at beginners then this is where mostly will be).

unless you specify a flag

https://msdn.microsoft.com/en-us/library/ee462497.aspx

Please .... look at the sf::Event implementation before saying that memcopy is not a valid way to copy it (explained in my book)

I know exactly how it is implemented (worked on it myself). Just because it is a union and contains only POD types doesn't mean its safe to copy with memcpy. There is a reason in C++ (you are not writing C) that copy constructors exist. Compilers are still free to add padding between members which will then screw over the copy. For the record, the event struct is more than just a union, its a union within a struct. As I said before you are copying your own classes with the copy constructors, so why do you insist on being ironic and copy the event improperly?

Lets be realistic, even if it was safe to do - this is a clear example of premature optimization regardless if you agree or not. If you ran the the entire program through a profiler with both ways of copying, you wouldn't even see a difference between the two.

On a side note, I can't read the book only from source code as I am not selected as a reviewer.

If you really think you can't copy a class because it doesn't have an explicit copy constructor think again.

I know that, but it's simpler for beginner because they don't have to download them one by one. In my book, I explain how to install (properly) all the dependencies. By the way it allow user to use the download from zip button, without having to do

Still don't get it do you? What is the point of submodules at all when you include the source directly? If the user decides to clone the repo they already got the extlibs source. Now they init the submodules and then they get 2 copies of extlibs.

You can't mix and match, either pick submodules or include all the source (not recommended at all).

I haven't stole the ship, I've find it under creative common, so .... where is the trouble?

Lets assume google is wrong and was unable to find the actual source of it... if you did find it under the CC license then you are still stealing it by not adhering to the license clauses. No matter which way you read that, you are required to give credit to wherever you got the assets from. (none of this is legal advice, but IMHO the owner has rights to come after you unless you follow the license [note a book is considered a commercial use]).

Let me say it this way, you can totally ignore all I have said, just don't think I am the only person that thinks this way.

Krozark commented 9 years ago

take a look here : http://opengameart.org/content/space-shooter-redux

zsbzsb commented 9 years ago

Fine, go ahead and close it and just ignore everything else. Artwork is just one of the things I mentioned. But of course nothing else matters because you are so amazing, good luck.

Krozark commented 9 years ago

As already said, the code don't have any release version for the moment, and some changes will be made. And some of them are those that you have speak about.So I don't ignore it at all. By the way, I don't appreciate the way you "share" them with me.