@Iwan good work on improving render.cpp however on low resolutions (4:3) the time text sizes are too big and distracting, while the rev and the map obstruct the player's view. Therefore I suggest that the time text be made smaller, and both the map and rev dial moved a bit closer to the edge.
About the font files, I am thinking to ditch the AltStyle idea with two fonts per image, and instead simply use a 2:1 image containing one font for each different font/style.
@Iwan also, I now want to share my ideas about the code style, seeing that I'm not the only active programmer around here. Let's discuss them now.
In the long run, I would like to convert all source code to use Allman 4-spaces tabs style, instead of the current K&R 2-spaces tab style.
I would also like to put a "proper" GNU GPL 2 notification, including the names of the people who contributed. To that effect I would ask you to add your name to the files you've changed.
Finally, we are working on another person's code. For me, it is already hard to follow because it doesn't contain much documentation in the form of comments. This is why I tend to add a lot of comments to code that I contribute (Doxygen style comments) and others' code that I successfully decipher. I think it would benefit us and future developers to have the code properly documented.
See audio.cpp for an example of the above suggestions, applied partially.
Back to code comments, Iwan you have recently written the following code:
// position of gear & speed number & label
glTranslatef( hratio * (1.f - (5.75f/50.f)) - 0.3f, -vratio * (40.f/50.f) + 0.2f, 0.0f);
Honestly I cannot understand what is going on. I would prefer you to document what screen percentages the above translation respects.
I also noticed that the gear dial and gear number aren't translated to the exact same position (one is 0.02f higher) but we'll revisit this later, after we decide what to do about AltStyle; I think we should use a 2:1 (2048x1024) texture containing a single font and scrap PTEXT_ALTSTYLE in the next version.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
on low resolutions (4:3) the time text sizes are too big and distracting
Please clarify: are all resolutions using 4:3 aspect ratio affected or are only the low resolutions affected? What is the first resolution that you would call "low"? 1024x768? Are the higher resolutions fine (1600x1200)? They should actually be equally affected but perhaps because of 1600x1200 screens usually being bigger it's less of a problem.
Or do you not like the style in general?
About the font files, I am thinking to ditch the AltStyle idea with two fonts per image, and instead simply use a 2:1 image containing one font for each different font/style.
I would suggest we use only the current two font styles (Droid Sans Mono with and without outline) and keep the implementation as is. What's the motivation to switch? You want more fonts?
In the long run, I would like to convert all source code to use Allman 4-spaces tabs style, instead of the current K&R 2-spaces tab style.
I like 2 spaces, I dislike 4 spaces, I hate tabs. I would suggest to keep whatever is in place. Ultimatively feel free to decide.
I would also like to put a "proper" GNU GPL 2 notification
I like the current one-liner but have no objection.
including the names of the people who contributed.
I think revision control systems made that superfluous but no objection. If you want to add names, you might want to check the svn log filename of each file, as render.cpp for examle only names jareiko but at least 5 users contributed by now in total.
To that effect I would ask you to add your name to the files you've changed.
Okay. You should clarify how to add though (another full Copyright 2014 line? add my name to the end separated by ";"?
This is why I tend to add a lot of comments to code that I contribute (Doxygen style comments) and others' code that I successfully decipher.
Absolutely! Awesome and yay for that! I have no idea how to do proper Doxygen comments since I hardly know anything about types in C++. Hopefully I'll be able to adapt when reading your commits' comments though.
// position of gear & speed number & label
glTranslatef( hratio * (1.f - (5.75f/50.f)) - 0.3f, -vratio * (40.f/50.f) + 0.2f, 0.0f);
Honestly I cannot understand what is going on. I would prefer you to document what screen percentages the above translation respects.
Oh, I thought I had added the more detailed comments in the first occurence of my changes to glTranslate(). My mistake. It starts in 1046.. and might be insufficient:
// time position (other time strings inherit this position)
// -hratio is left border, 0 is center, +hratio is right
// hratio * (1/50) gives 1% of the entire width
// +vratio is top border, 0 is middle, -vratio is bottom
glTranslatef( -hratio + hratio * (5.75f/50.f), vratio * (4.f/5.f), 0.0f);
So I figured out that glTranslatef (not defined inside trigger but in GL it seems) moves things around the screen. I'm not sure why but -hratio means left border and hratio means right border, vratio means top and -vratio means bottom. From that I concluded that hratio/50 equals 1% of the screen width. "-hratio + hratio * (5.75f/50.f)" means "5.75% right of the left border.
Would it be a solution to move this comment to the first occurrence of my %-calculation method and adapt to the way it's used there? Or do you have another idea how this should be documented or even implemented?
I got stuck for a few minutes when I tried to calculate "4/5" and got "1". After that I added .f to all the numbers in all functions I changed where decimal numbers are used. That might have been overkill, suggestions are very welcome.
I also noticed that the gear dial and gear number aren't translated to the exact same position (one is 0.02f higher).
This is because the new font has a higher center point compared to the previous font.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Please clarify: are all resolutions using 4:3 aspect ratio affected or are only the low resolutions affected?
It seems that they are, because things are scaled up proportionally.
Or do you not like the style in general?
I guess you can say that, although this isn't as much style as it is screen space usage. I like the new rev dial that we made and I have no objection to the TIME and CKPT labels that were added. However I think the big timer font wastes screen space and is distracting, while the map and rev dial are too close to the center of the screen, feeling needlessly crammed. So, I have no problem with their size (unlike the timer) but I do have a problem with their positioning.
I would suggest we use only the current two font styles (Droid Sans Mono with and without outline) and keep the implementation as is. What's the motivation to switch? You want more fonts?
Yes I do. I am very much looking forward to new shadowy fonts, like the ones you originally made (but which unfortunately had a shadow cutoff problem). Besides that, the current implementation just doesn't feel right. One file, one font: this is how it should be. Changing the fonts in the game code will be as simple as calling tex_outline_font->bind(); which has the benefit of clarity over PTEXT_ALTSTYLE which you don't know what will give you unless you know the data files in and out.
I like 2 spaces, I dislike 4 spaces, I hate tabs. I would suggest to keep whatever is in place. Ultimatively feel free to decide.
Just to be clear, I do not want to use hard tabs. What I meant was to have tabs inserted as 4 spaces instead of 2. OK then, I will proceed to use 4 spaces.
If you want to add names, you might want to check the svn log filename of each file, as render.cpp for examle only names jareiko but at least 5 users contributed by now in total.
I won't do that. Everybody is (was) responsible for adding their own names as contributors. I am not a lawyer but it seems the formal way to "say you contributed" is by way of adding a copyright with your name. If I am in the wrong about this, please let me know.
Okay. You should clarify how to add though (another full Copyright 2014 line? add my name to the end separated by ";"?
Again I am not a lawyer but I think adding your own "Copyright (c) 2014-2015" line is the proper way to do this.
I have no idea how to do proper Doxygen comments since I hardly know anything about types in C++.
Doxygen comments don't have much to do with C++ types, or even with C++ itself. They're used in a variety of programming languages to have the program "doxygen" automatically generate documentation by scanning the source code, but they're also very good for any humans reading the code. So if you ever write your own function, just add a Doxygen comment like this at the beginning:
///
/// @brief Purpose of the function.
/// @details More detailed explanation possibly spanning
/// multiple lines if needed.
/// @param p What parameter p is for.
/// @param q What parameter q is for... and so on.
/// @returns What is the general meaning of the result.
/// @retval x Any special value x for the result, if any, with its meaning.
///
So I figured out that glTranslatef (not defined inside trigger but in GL it seems) moves things around the screen. I'm not sure why but -hratio means left border and hratio means right border, vratio means top and -vratio means bottom. From that I concluded that hratio/50 equals 1% of the screen width. "-hratio + hratio * (5.75f/50.f)" means "5.75% right of the left border.
OK. To clarify, the game calculates hratio and vratio like this:
screen is 4:3 -> hratio=4/3 vratio=1
screen is 16:9 -> hratio=16/9 vratio=1
screen is 2:5 -> hratio=1 vratio=5/2
screen is 3:10 -> hratio=1 vratio=10/3
So, as you already figured out, this means:
( 0, 0) -> center of screen
(-hratio, -vratio) -> bottom left
(-hratio, vratio) -> top left
( hratio, vratio) -> top right
( hratio, -vratio) -> bottom right
[...] Or do you have another idea how this should be documented or even implemented?
Right now I do not have ideas how to improve the meaning of the code. You could try to break it down and simplify it by adding variables which you document individually:
I am leaving this up to you in case you decide to also move the map and rev dial further away from the center of the screen.
This is because the new font has a higher center point compared to the previous font.
Isn't this fixable by centering the fonts in the PNG instead of compensating for it in code? If the letters are already centered inside the texture then they will be automatically centered in the game as well. Maybe there is something I misunderstand? Have you tried drawing boxes around the letters to see if the game code cuts them in the space place? If the code does something unexpected let me know.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
@Iwan good work on improving render.cpp however on low resolutions (4:3) the time text sizes are too big and distracting, while the rev and the map obstruct the player's view. Therefore I suggest that the time text be made smaller, and both the map and rev dial moved a bit closer to the edge.
About the font files, I am thinking to ditch the AltStyle idea with two fonts per image, and instead simply use a 2:1 image containing one font for each different font/style.
@Iwan also, I now want to share my ideas about the code style, seeing that I'm not the only active programmer around here. Let's discuss them now.
In the long run, I would like to convert all source code to use Allman 4-spaces tabs style, instead of the current K&R 2-spaces tab style.
I would also like to put a "proper" GNU GPL 2 notification, including the names of the people who contributed. To that effect I would ask you to add your name to the files you've changed.
Finally, we are working on another person's code. For me, it is already hard to follow because it doesn't contain much documentation in the form of comments. This is why I tend to add a lot of comments to code that I contribute (Doxygen style comments) and others' code that I successfully decipher. I think it would benefit us and future developers to have the code properly documented.
See audio.cpp for an example of the above suggestions, applied partially.
Back to code comments, Iwan you have recently written the following code:
Honestly I cannot understand what is going on. I would prefer you to document what screen percentages the above translation respects.
I also noticed that the gear dial and gear number aren't translated to the exact same position (one is 0.02f higher) but we'll revisit this later, after we decide what to do about AltStyle; I think we should use a 2:1 (2048x1024) texture containing a single font and scrap
PTEXT_ALTSTYLEin the next version.Please clarify: are all resolutions using 4:3 aspect ratio affected or are only the low resolutions affected? What is the first resolution that you would call "low"? 1024x768? Are the higher resolutions fine (1600x1200)? They should actually be equally affected but perhaps because of 1600x1200 screens usually being bigger it's less of a problem.
Or do you not like the style in general?
I would suggest we use only the current two font styles (Droid Sans Mono with and without outline) and keep the implementation as is. What's the motivation to switch? You want more fonts?
I like 2 spaces, I dislike 4 spaces, I hate tabs. I would suggest to keep whatever is in place. Ultimatively feel free to decide.
I like the current one-liner but have no objection.
I think revision control systems made that superfluous but no objection. If you want to add names, you might want to check the
svn log filenameof each file, as render.cpp for examle only names jareiko but at least 5 users contributed by now in total.Okay. You should clarify how to add though (another full Copyright 2014 line? add my name to the end separated by ";"?
Absolutely! Awesome and yay for that! I have no idea how to do proper Doxygen comments since I hardly know anything about types in C++. Hopefully I'll be able to adapt when reading your commits' comments though.
Here are your changes to audio.cpp: https://sourceforge.net/p/trigger-rally/code/134/tree//src/pengine/audio.cpp?diff=42 I doubt I will comment function definitions any time soon but I think I understand what I see so far.
Oh, I thought I had added the more detailed comments in the first occurence of my changes to glTranslate(). My mistake. It starts in 1046.. and might be insufficient:
So I figured out that glTranslatef (not defined inside trigger but in GL it seems) moves things around the screen. I'm not sure why but -hratio means left border and hratio means right border, vratio means top and -vratio means bottom. From that I concluded that hratio/50 equals 1% of the screen width. "-hratio + hratio * (5.75f/50.f)" means "5.75% right of the left border.
Would it be a solution to move this comment to the first occurrence of my %-calculation method and adapt to the way it's used there? Or do you have another idea how this should be documented or even implemented?
I got stuck for a few minutes when I tried to calculate "4/5" and got "1". After that I added .f to all the numbers in all functions I changed where decimal numbers are used. That might have been overkill, suggestions are very welcome.
This is because the new font has a higher center point compared to the previous font.
It seems that they are, because things are scaled up proportionally.
I guess you can say that, although this isn't as much style as it is screen space usage. I like the new rev dial that we made and I have no objection to the TIME and CKPT labels that were added. However I think the big timer font wastes screen space and is distracting, while the map and rev dial are too close to the center of the screen, feeling needlessly crammed. So, I have no problem with their size (unlike the timer) but I do have a problem with their positioning.
Yes I do. I am very much looking forward to new shadowy fonts, like the ones you originally made (but which unfortunately had a shadow cutoff problem). Besides that, the current implementation just doesn't feel right. One file, one font: this is how it should be. Changing the fonts in the game code will be as simple as calling
tex_outline_font->bind();which has the benefit of clarity overPTEXT_ALTSTYLEwhich you don't know what will give you unless you know the data files in and out.Just to be clear, I do not want to use hard tabs. What I meant was to have tabs inserted as 4 spaces instead of 2. OK then, I will proceed to use 4 spaces.
I won't do that. Everybody is (was) responsible for adding their own names as contributors. I am not a lawyer but it seems the formal way to "say you contributed" is by way of adding a copyright with your name. If I am in the wrong about this, please let me know.
Again I am not a lawyer but I think adding your own "Copyright (c) 2014-2015" line is the proper way to do this.
Doxygen comments don't have much to do with C++ types, or even with C++ itself. They're used in a variety of programming languages to have the program "doxygen" automatically generate documentation by scanning the source code, but they're also very good for any humans reading the code. So if you ever write your own function, just add a Doxygen comment like this at the beginning:
OK. To clarify, the game calculates
hratioandvratiolike this:So, as you already figured out, this means:
Right now I do not have ideas how to improve the meaning of the code. You could try to break it down and simplify it by adding variables which you document individually:
I am leaving this up to you in case you decide to also move the map and rev dial further away from the center of the screen.
Isn't this fixable by centering the fonts in the PNG instead of compensating for it in code? If the letters are already centered inside the texture then they will be automatically centered in the game as well. Maybe there is something I misunderstand? Have you tried drawing boxes around the letters to see if the game code cuts them in the space place? If the code does something unexpected let me know.