Venture Adventure - fix level numbering

[MHLoppy]

Description

Right now the max level check works on a +1 offset: when the value is 3, the game supports two levels, not three.

This seems extremely unintuitive, so I've changed it so that the max level check equals the number of levels loaded (when the value is 2, the game will load two levels).

Note: this PR builds off of the "small refactor" PR sequence from a few weeks ago.

Type of change

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

I recompiled the game and played through the available levels, ensuring that the number of accessible levels remained the same.

Testing Checklist

• Played through all of level 1
• Played through all of level 2 to cycle to the main menu

Checklist

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I'll notify my team when the PR is ready to review

[MHLoppy]

(p.s. there might still be some minor fixes to apply even after this, #218, and #219 are applied, as it's not hard to imagine that some of the automated merging by git introduced silent incompatibilities between my original PR and Castiel's older T1 PR which caused the conflicts)

[Bullet-Not-Proof]

strange choice in the original version this makes a lot more sense.

[lexlam1524]

Just took a look at your pull request and wanted to share some thoughts.

One thing that caught my eye was the duplicated code in the new_player() function and the Player class constructor. Would be awesome if we could simplify that and remove the unused Player class altogether.

Also, noticed some magic numbers floating around (like 32, 4, and 12). Would be super helpful if we could define some constants or enums to explain what those numbers mean. Makes the code way easier to read and understand!

const int BITMAP_WIDTH = 32;
const int BITMAP_HEIGHT = 32;
const int CELL_DETAILS_X = 4;
const int CELL_DETAILS_Y = 12;

Lastly, the game loop is looking a bit complex. Would love to see it broken down into smaller, more manageable chunks. Maybe we could create some separate functions or classes to handle specific tasks? Would make the code way more organized and easier to maintain.

void process_events();
void update_game_state();
void draw_game();
void handle_level_completion();
void handle_game_over();
void handle_debugging_output();

All in all, great work on this pull request! Just a few tweaks and we'll be golden. Looking forward to seeing the updated code!

[MHLoppy]

Just took a look at your pull request and wanted to share some thoughts.

I generally agree with your suggestions, but none of them are within the scope of the changes already made in the existing PR(s). What you've provided isn't really a review of the pull request(s), but instead suggestions on what could be done next if I were to do more work similar to what I've already done.

As you would've already seen in our team meetings and team comms, I decided that it was more efficient to rewrite the codebase in "Venture Adventure 2" than to individually resolve all of the individual problems remaining in the VA1 codebase (your suggestions would be good for followup work if the codebase was still being worked on).

[MHLoppy]

Note for us later:

If #228 ends up having bundle problems, it's probably due to missing #219 - at this stage I don't 100% remember what the interactions / dependencies are between the month-old PRs.

[lexlam1524]

ohh sorry about that. I think I just carried over the review style from last trimester, where we dug into code bugs and improvements. Thanks for telling me that!
I'll make sure to focus on the existing PRs from now on, rather than suggesting new stuff. Appreciate the feedback!

[MHLoppy]

Including suggestions for future work is fine (and in most cases helpful!), but you need to also look at the actual PR contents itself as that's the core purpose of us doing reviews on PRs.

I've looked through some of your PRs from last semester (e.g., #56, #67), in case this was just a case of teams doing things differently, and you'll see that the people reviewing your PRs are actually talking about things which directly relate to your PRs.

It's quite unexpected that you've completed the previous semester with a differing idea of the peer review process, but hopefully you have a better idea of it now? If you're not sure about something try to bring it up with team members early so that you can get clarification on it sooner rather than later (learning this at the end of your senior semester doesn't do you or us much good!). I had already brought this up a little in #224, so there's been a lot of time to follow up on the process if you still weren't sure about it.

[AmberPotion]

@lexlam1524 Just adding to @MHLoppy 's comment about your feedback in #228 (comment)

Please keep the feedback strictly to the actual code that was changed in this PR. The commit message is about level numbering, so comments about Player class, magic numbers, and overall game loop is irrelevant to this PR.

For the stuff written in your feedback: You should be either 1) do these changes yourself as a person in the dev team as well and submit a new PR, or 2) coordinating with the team by putting these tasks into the planning tools that are used by the team at the moment i.e. Planner.