PascalGameDevelopment / SDL2-for-Pascal

Unit files for building Free Pascal and Delphi applications using the SDL2 library
https://pascalgamedevelopment.github.io/SDL2-for-Pascal/
Mozilla Public License 2.0
103 stars 20 forks source link

Added all missing double-pointer types in all source files #122

Closed flowCRANE closed 1 year ago

flowCRANE commented 1 year ago

When it comes to GitHub, and especially contributing to open source, I have two left hands — I don't really understand how to click it correctly, so it's a big success anyway that the repository is still working. :)

suve commented 1 year ago

Ehh. Personally I'm really not a fan of this, but let's not repeat the discussion from #105 all over again.

There are some places where the indentation doesn't match, but if @Free-Pascal-meets-SDL-Website doesn't mind, I can merge this and fix misaligned places.

flowCRANE commented 1 year ago

Indentations match everywhere, according to the types they are pointers to. I've checked this in both the IDE and GitHub.

Ehh. Personally I'm really not a fan of this, but let's not repeat the discussion from #105 all over again.

Even if these types are not used by headers, they will definitely be used by header users. The longer I developed my game code, the more double pointers I missed. Why would I declare them myself in the game code, when these types are strictly headers-related and should be provided by them? Well, that's why it's definitely better to include them in the headers and let everyone use what they need. At least these types will be properly declared and will always match the base header types.

If more types are added to the headers in the future, I suggest declaring a single and a double pointer for each of them. This way the headers will be fully usable and well organized, which is very important.

Free-Pascal-meets-SDL-Website commented 1 year ago

There are some places where the indentation doesn't match, but if @Free-Pascal-meets-SDL-Website doesn't mind, I can merge this and fix misaligned places.

Damn, missed these, saw them just on second inspection! Great job! I guess @furious-programming should have the chance to fix this as this is his/her PR.

Indentations match everywhere, according to the types they are pointers to. I've checked this in both the IDE and GitHub.

There are some mismatching indentations, see e. g. TSDL_SpinLock. Could you fix them, please.

If more types are added to the headers in the future, I suggest declaring a single and a double pointer for each of them. This way the headers will be fully usable and well organized, which is very important.

Please add as new fifth point (right before the point that links to the Tranlsation Cheat Sheet) in section Code style guidelines: For convenience we encourage to add single and double pointers for any SDL type. (See #105)

flowCRANE commented 1 year ago

There are some mismatching indentations, see e. g. TSDL_SpinLock. Could you fix them, please.

I didn't even touch the TSDL_SpinLock declaration, so if the indentation is incorrect, it was incorrect all the time.

Edit: the indentation is incorrect, because one of the contributors uses Tabs instead of Spaces and the indentation is mixed. This is not visible in the Lazarus, since the IDE replace tabulators to spaces, but on GitHub this is visible. I'm using Spaces only (two for every indentation level), the changes made by me (concerning this pull request) was made in Lazarus (not in GitHub editor), so it's not my fault.

Free-Pascal-meets-SDL-Website commented 1 year ago

Thanks, from my side this is ready for merge now :-). Please just merge, if you agree @suve.

suve commented 1 year ago

I took the liberty to rebase the changes on top of latest master and fix any indentation issues I've found. If you say it looks okay now, then I'm going to merge it.