Document functions, add features: 'return to menu', 'remove tiles' #145

BurhanHTW · 2024-06-19T16:28:55Z

Implemented 'Return to Menu' functionality.
Added 'Remove Tiles' feature.
Unified saving game state and statistics into a single file.
Enabled selection of saved game states for continuation.

- Implemented 'Return to Menu' functionality. - Added 'Remove Tiles' feature. - Unified saving game state and statistics into a single file. - Enabled selection of saved game states for continuation.

BurhanHTW · 2024-06-19T16:30:51Z

We have submitted the pull request, and it has been a pleasure to contribute to this project.

plibither8

Code looks good, thank you very much! :)

I can see that a CI workflow is failing. Could you check the error behind it? Ideally, we want these checks to be successful before trusting it to be merged.

Additionally, a couple of changes I'd love to see before merging it:

Let's remove the .vscode directory. More details in the review comments.
Let's be uniform with how we do in-line comments. Gives much better readability.
The added code would benefit from being run against a formater/linter. This will format the code to make it uniform. We've got a .clang-format file that specifies the formatting conventions used in this project.

Nevertheless, I'm approving this PR on the basis of the solid feature code additions -- they look great!

plibither8 · 2024-06-22T04:36:51Z

src/.vscode/c_cpp_properties.json

@@ -0,0 +1,18 @@
+{


Let's remove the .vscode files. They're meant for developer-specific configs on local machines, so shouldn't be part of the repository.

Bonus: Add the .vscode folder to the .gitignore file. This should prevent it from being staged and committed in the future as well! :)

plibither8 · 2024-06-22T04:37:08Z

src/.vscode/launch.json

@@ -0,0 +1,24 @@
+{


Same with this file... let's remove it.

plibither8 · 2024-06-22T04:37:15Z

src/.vscode/settings.json

@@ -0,0 +1,114 @@
+{


...and this one too

plibither8 · 2024-06-22T04:37:38Z

src/game-graphics.cpp

+ * @note The new addition includes 'M' for returning to the menu.
+ * 
+ * @return std::string A formatted string containing the list of input commands.
+ */


Comments look great, thank you!

plibither8 · 2024-06-22T04:38:43Z

src/game-graphics.cpp

+ * The commands include movements (Up, Left, Down, Right), saving the game, and returning to the menu.
+ * The prompt is formatted with indentation for readability.
+ * 
+ * @note The new addition includes 'M' for returning to the menu.


We can remove this note, since it will get outdated pretty quickly.

plibither8 · 2024-06-22T04:45:37Z

src/gameboard.cpp

@@ -135,7 +149,7 @@ bool canMoveOnGameboardDataArray(gameboard_data_array_t gbda) {
      const auto offset_check = {
          current_point + offset, // Positive adjacent check
          current_point - offset}; // Negative adjacent Check
-      for (const auto current_offset : offset_check) {
+      for (const auto &current_offset : offset_check) { //auto REFERENCE current_offset


commented out code 👀

plibither8 · 2024-06-22T04:47:34Z

src/loadresource.cpp

+
+  while (std::getline(stateFile, tempLine)) {
+      if (tempLine.find('[') != std::string::npos) {
+          break; // Exit loop if '[' is found


Great idea here, as well as the -1 to return an error!

plibither8 · 2024-06-22T04:48:11Z

src/loadresource.cpp

@@ -81,7 +128,7 @@ get_and_process_game_stats_string_data(std::istream &stats_file) {
  if (stats_file) {
    unsigned long long score;
    long long moveCount;
-    for (std::string tempLine; std::getline(stats_file, tempLine);) {
+    for (std::string tempLine; std::getline(stats_file, tempLine, '.');) {


Why was the '.' required?

plibither8 · 2024-06-22T04:49:12Z

src/menu.cpp

+    mainmenustatus[FLAG_START_MENU] = true; // Initial state is Menu
+    if (mainmenustatus[FLAG_START_MENU] == true)    // If the menu flag is set, we enter...
+    {
+      soloLoop();                         // ... the soloLoop() function, where we navigate the menu


Let's move these comments to the line above.

plibither8 · 2024-06-22T04:49:39Z

src/scores-graphics.cpp

@@ -56,7 +56,7 @@ std::string ScoreboardOverlay(scoreboard_display_data_list_t sbddl) {
             << "\n";
    };

-    for (const auto s : sbddl) {
+    for (const auto& s : sbddl) { // auto REFERENCE s


commented out code 👀

BurhanHTW · 2024-06-22T06:52:31Z

We‘ll try to fix the CI. Hopefully this solved asap :)

BurhanHTW · 2024-06-22T07:35:57Z

I have to switch from C++14 to C++17 in CMakeLists.txt #include in menu.cpp is only supported from C++17 onwards. https://en.cppreference.com/w/cpp/filesystem

plibither8 · 2024-06-22T07:38:21Z

I have to switch from C++14 to C++17 in CMakeLists.txt #include in menu.cpp is only supported from C++17 onwards. https://en.cppreference.com/w/cpp/filesystem

Have you tried fixing the issue using the suggestion in the error message? std::_fs::filesystem

BurhanHTW · 2024-06-22T08:07:56Z

I have to switch from C++14 to C++17 in CMakeLists.txt #include in menu.cpp is only supported from C++17 onwards. https://en.cppreference.com/w/cpp/filesystem

Have you tried fixing the issue using the suggestion in the error message? std::_fs::filesystem

Not yet. But it should fixed by changing C14 to C17.

plibither8 · 2024-06-22T13:48:22Z

@BurhanHTW could you push the changes to your branch to re-trigger the CI?

BurhanHTW · 2024-06-22T14:05:43Z

@BurhanHTW could you push the changes to your branch to re-trigger the CI?

I will Push it later when i am at home. We have changed everything what you wanted :)

BurhanHTW · 2024-06-22T16:47:11Z

I cant push anything because the access is denied

BurhanHTW · 2024-06-22T17:04:47Z

I had to generate a new Token to push. Now we are waiting for the workflow approval. You can see our next commit as i can see

plibither8 · 2024-06-22T17:30:01Z

@BurhanHTW your latest commit has many merge conflicts present. Example

Remove all merge markers - they're causing the builds to fail.

BurhanHTW · 2024-06-22T18:03:59Z

@BurhanHTW your latest commit has many merge conflicts present. Example

Remove all merge markers - they're causing the builds to fail.

hopefully done :D

BurhanHTW · 2024-06-22T18:29:05Z

I don’t know what i have Done actually i have to Check the Problems …

BurhanHTW · 2024-06-22T18:57:09Z

I have to many Problems Right now i try to solve them.

BurhanHTW · 2024-06-22T19:43:50Z

If i have to many Problems. Is it possible to make a new pull Request? I guess i had to many Problems because i commited to many files

BurhanHTW · 2024-06-23T07:55:12Z

If the workflow fails again i‘ll create a new Pull Request and Push the latest Code again. I am sorry to have those issues. :/

BurhanHTW · 2024-06-24T05:46:51Z

So the workflow failed again. I will create a new pull Request.

plibither8 · 2024-06-24T05:48:04Z

Yep, that sounds like a good idea!

…

On 24-Jun-2024, at 11:17, BurhanHTW ***@***.***> wrote: So the workflow failed again. I will create a new pull Request. — Reply to this email directly, view it on GitHub <#145 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHTCX66PS62KPHAKA7IGWULZI6XGBAVCNFSM6AAAAABJSMEXOCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBVGY3DANBZGI>. You are receiving this because you commented.

BurhanHTW · 2024-06-24T06:03:11Z

I have closed this pull request. I will create a new one now.

Documented all functions and added new features:

dc326b5

- Implemented 'Return to Menu' functionality. - Added 'Remove Tiles' feature. - Unified saving game state and statistics into a single file. - Enabled selection of saved game states for continuation.

plibither8 approved these changes Jun 22, 2024

View reviewed changes

plibither8 changed the title ~~Documented all functions and added new features:~~ Document functions, add features: 'return to menu', 'remove tiles' Jun 22, 2024

Resolved merge conflicts in menu.cpp

7a6cd24

BurhanHTW force-pushed the new-features branch from 054d427 to 7a6cd24 Compare June 22, 2024 18:03

BurhanHTW added 3 commits June 22, 2024 20:41

Update game-graphics.cpp deleted the markers

4b2e974

Update gameboard.cpp removed markers

85dd5ff

Update scores-graphics.cpp deleted markers

29de2cd

Update saveresource.cpp deleted marker and Double Function

894e172

BurhanHTW closed this Jun 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Document functions, add features: 'return to menu', 'remove tiles' #145

Document functions, add features: 'return to menu', 'remove tiles' #145

BurhanHTW commented Jun 19, 2024

BurhanHTW commented Jun 19, 2024

plibither8 left a comment

plibither8 Jun 22, 2024

plibither8 Jun 22, 2024

plibither8 Jun 22, 2024

plibither8 Jun 22, 2024

plibither8 Jun 22, 2024

plibither8 Jun 22, 2024

plibither8 Jun 22, 2024

plibither8 Jun 22, 2024

plibither8 Jun 22, 2024

plibither8 Jun 22, 2024

BurhanHTW commented Jun 22, 2024

BurhanHTW commented Jun 22, 2024

plibither8 commented Jun 22, 2024

BurhanHTW commented Jun 22, 2024 •

edited

Loading

plibither8 commented Jun 22, 2024

BurhanHTW commented Jun 22, 2024

BurhanHTW commented Jun 22, 2024

BurhanHTW commented Jun 22, 2024 •

edited

Loading

plibither8 commented Jun 22, 2024

BurhanHTW commented Jun 22, 2024

BurhanHTW commented Jun 22, 2024

BurhanHTW commented Jun 22, 2024 •

edited

Loading

BurhanHTW commented Jun 22, 2024

BurhanHTW commented Jun 23, 2024

BurhanHTW commented Jun 24, 2024

plibither8 commented Jun 24, 2024 via email

BurhanHTW commented Jun 24, 2024

Document functions, add features: 'return to menu', 'remove tiles' #145

Document functions, add features: 'return to menu', 'remove tiles' #145

Conversation

BurhanHTW commented Jun 19, 2024

BurhanHTW commented Jun 19, 2024

plibither8 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BurhanHTW commented Jun 22, 2024

BurhanHTW commented Jun 22, 2024

plibither8 commented Jun 22, 2024

BurhanHTW commented Jun 22, 2024 • edited Loading

plibither8 commented Jun 22, 2024

BurhanHTW commented Jun 22, 2024

BurhanHTW commented Jun 22, 2024

BurhanHTW commented Jun 22, 2024 • edited Loading

plibither8 commented Jun 22, 2024

BurhanHTW commented Jun 22, 2024

BurhanHTW commented Jun 22, 2024

BurhanHTW commented Jun 22, 2024 • edited Loading

BurhanHTW commented Jun 22, 2024

BurhanHTW commented Jun 23, 2024

BurhanHTW commented Jun 24, 2024

plibither8 commented Jun 24, 2024 via email

BurhanHTW commented Jun 24, 2024

BurhanHTW commented Jun 22, 2024 •

edited

Loading

BurhanHTW commented Jun 22, 2024 •

edited

Loading

BurhanHTW commented Jun 22, 2024 •

edited

Loading