lethal-guitar / RigelEngine

A modern re-implementation of the classic DOS game Duke Nukem II
GNU General Public License v2.0
920 stars 60 forks source link

Made Viewport consistent throughout #812

Closed mohitsaxenaknoldus closed 2 years ago

mohitsaxenaknoldus commented 2 years ago

Fixes #795

lethal-guitar commented 2 years ago

Thanks for the PR, but I actually had something different in mind. #795 is about the names of identifiers in code, e.g. variables, parameters, function names etc. Where viewport is spelled as two camel-case words, i.e. viewPort or ViewPort, I'd like it to be spelled as viewport or Viewport instead. Otherwise, spelling or casing doesn't need to change, so e.g. viewport can stay as is.

Here is an example:

https://github.com/lethal-guitar/RigelEngine/blob/c1da6f88da0d676feef955629121f308f039df60/src/game_logic/camera.cpp#L115

The parameter viewPortSize should be called viewportSize instead in this case.

Hope this clarifies it, let me know if you have any questions!

mohitsaxenaknoldus commented 2 years ago

Thanks for the PR, but I actually had something different in mind. #795 is about the names of identifiers in code, e.g. variables, parameters, function names etc. Where viewport is spelled as two camel-case words, i.e. viewPort or ViewPort, I'd like it to be spelled as viewport or Viewport instead. Otherwise, spelling or casing doesn't need to change, so e.g. viewport can stay as is.

Here is an example:

https://github.com/lethal-guitar/RigelEngine/blob/c1da6f88da0d676feef955629121f308f039df60/src/game_logic/camera.cpp#L115

The parameter viewPortSize should be called viewportSize instead in this case.

Hope this clarifies it, let me know if you have any questions!

I am still confused about the requirements. Can you please explain again?

lethal-guitar commented 2 years ago

I am still confused about the requirements. Can you please explain again?

Sure. Ok, so let's look at the file src/game_logic/camera.cpp as an example. In that file, on line 115, a parameter named viewPortSize is declared. This parameter should be named viewportSize instead.

The name viewPortSize in camel-case reads as "view port size", with "view" and "port" being two separate words due to the capitalization of "Port". But "viewport" is actually a single word, so the correct camel-case spelling for the words "viewport size" is viewportSize. (I didn't know that "viewport" is a single word when I was writing a lot of this code).

This applies to all variables, function names etc. in the code. Whenever "viewport" is a part of the name, then the variable/function/... needs to be renamed so that "viewport" reads as a single word, by making the "P" in "port" lowercase. It doesn't matter what other words are part of the name, everything else remains unchanged, it's just about making the "P" lowercase whenever viewPort or ViewPort appears in the code

Let's look at another example. In the file upscaling_utils.hpp, a struct named WidescreenViewPortInfo is declared. Again, we need to make "viewport" read as one word, so the struct should instead be called WidescreenViewportInfo.

And then the same for any other identifier in the code that is or contains viewPort (or ViewPort).

mohitsaxenaknoldus commented 2 years ago

I am still confused about the requirements. Can you please explain again?

Sure. Ok, so let's look at the file src/game_logic/camera.cpp as an example. In that file, on line 115, a parameter named viewPortSize is declared. This parameter should be named viewportSize instead.

The name viewPortSize in camel-case reads as "view port size", with "view" and "port" being two separate words due to the capitalization of "Port". But "viewport" is actually a single word, so the correct camel-case spelling for the words "viewport size" is viewportSize. (I didn't know that "viewport" is a single word when I was writing a lot of this code).

This applies to all variables, function names etc. in the code. Whenever "viewport" is a part of the name, then the variable/function/... needs to be renamed so that "viewport" reads as a single word, by making the "P" in "port" lowercase. It doesn't matter what other words are part of the name, everything else remains unchanged, it's just about making the "P" lowercase whenever viewPort or ViewPort appears in the code

Let's look at another example. In the file upscaling_utils.hpp, a struct named WidescreenViewPortInfo is declared. Again, we need to make "viewport" read as one word, so the struct should instead be called WidescreenViewportInfo.

And then the same for any other identifier in the code that is or contains viewPort (or ViewPort).

Please check now.

lethal-guitar commented 2 years ago

@mohitsaxenaknoldus I've edited my review comment above and added some more examples, hope this helps. Sorry that the issue description wasn't so clear, I'll try to give more examples next time.