3d moment#17

Open

ShahriarAhnaf wants to merge 7 commits intomainfrom

Owner

ShahriarAhnaf commented Mar 31, 2026

working 27 cube render with movement

ShahriarAhnaf added 3 commits

March 29, 2026 13:08


          Doing a index based inner loop search to fix the performance bottle n…

304e5d7

…eck. Several correctness fixed from adding tests.


          fixing animation!

9e26c77


          little bit of trolling to connect both. very easy to hang this though..

2bd3f00

ShahriarAhnaf requested a review from Copilot

March 31, 2026 20:12

Copilot started reviewing on behalf of ShahriarAhnaf

March 31, 2026 20:12

Copilot AI reviewed

View reviewed changes

Copilot AI left a comment

Pull request overview

This PR upgrades the project from a single-cube demo to a functional Rubik’s Cube visualizer with 26 “cubies”, animated face turns, and basic scramble/solve controls, while also introducing a unified top-level CMake build that separates core cube/solver logic from the visual engine.

Changes:

Implemented per-cubie rendering with move queue + 90° animated slice rotations and orbit camera controls.
Refactored/expanded cube + solver logic (move indexing, faster IDFS path building, additional solve stages) and added a new Core/ static library.
Reworked build/test setup (top-level CMake, removed generated CMake artifacts, expanded .gitignore, updated shader inputs and test suite).

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
Visual Engine/src/CubeRenderer.cpp	New 26-cubie dynamic mesh build per frame, move animation/queue, scramble/solve input handling, orbit camera callbacks.
Visual Engine/include/CubeRenderer.h	New cubie/animation data structures and integration with `RubixCube` + `Solver`.
Visual Engine/shaders/cube.vert	Uses explicit `layout(location=...)` attributes for VAO binding consistency.
Visual Engine/shaders/cube.frag	Two-sided-ish lighting tweak (flipped normals) + updated lighting constants.
Rubix/Testing/Test_Cube.cpp	Replaced interactive ncurses test with a non-interactive test suite and more move/solver checks.
Rubix/src/Solver.cpp	Introduced `Solve_DFS_fast` + IDFS path building and updated staged solve approach.
Rubix/src/Cube.cpp	Fixes/normalizes some turn implementations and adds `apply_move_index` + `*2` parsing in `apply_moves`.
Rubix/Inc/Solver.h	Adds “relevant mask” helpers and switches DFS/IDFS interfaces to the faster indexed path approach.
Rubix/Inc/Cube.h	Declares `apply_move_index`.
Core/src/Solver.cpp	New core solver implementation used by unified build (`CubeCore`).
Core/src/Cube.cpp	New core cube implementation used by unified build (`CubeCore`).
Core/Inc/Solver.h	New core solver API + target-state masking utilities.
Core/Inc/Cube.h	New core cube API + constants/enums (no ncurses dependency).
CMakeLists.txt	New unified build: `CubeCore` + visualizer + tests, FetchContent deps, shader copy step, run targets.
.gitignore	Expanded ignores for build outputs, CMake files, IDE artifacts, etc.
Visual Engine/CMakeCache.txt	Removed generated build artifact from repo.
Visual Engine/cmake_install.cmake	Removed generated build artifact from repo.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Visual Engine/src/CubeRenderer.cpp

Comment on lines 17 to 20

               {
-                  cubeState.model = glm::mat4(1.0f);
-                  cubeState.rotation = glm::vec3(0.0f);
-                  cubeState.scale = 1.0f;
+                  memset(keyStates, 0, sizeof(keyStates));
+                  currentAnim = {};
               }

Copilot AI Mar 31, 2026

memset is used but <cstring> isn't included in this translation unit (and it isn't included via the header either), which can cause a compile error depending on the standard library. Add #include <cstring> (or replace with std::fill(std::begin(keyStates), std::end(keyStates), false) and include <algorithm>).

Copilot uses AI. Check for mistakes.

Owner Author

ShahriarAhnaf Mar 31, 2026

@copilot apply changes based on this feedback

Visual Engine/src/CubeRenderer.cpp

Comment on lines +467 to +472

+                          std::string solution = solver.Solve_Cube(copy, 8);
+                          if (!solution.empty()) {
+                              enqueueMoveString(solution);
+                              std::cout << "Solution: " << solution << std::endl;
+                          } else {
+                              std::cout << "No solution found within depth limit" << std::endl;

Copilot AI Mar 31, 2026

When the cube is already solved, Solve_Cube() can legitimately return an empty string (0 moves). This branch treats an empty solution as failure and prints "No solution found..."; consider distinguishing "already solved" from "unsolved and not found" (e.g., check copy == RubixCube() before solving, or have the solver return an explicit status).

Suggested change

      
                        std::string solution = solver.Solve_Cube(copy, 8);
          
                        if (!solution.empty()) {
          
                            enqueueMoveString(solution);
          
                            std::cout << "Solution: " << solution << std::endl;
          
                        } else {
          
                            std::cout << "No solution found within depth limit" << std::endl;
          
                        // If the cube is already solved, do not treat it as "no solution found"
          
                        if (copy == RubixCube()) {
          
                            std::cout << "Cube is already solved (0 moves needed)." << std::endl;
          
                        } else {
          
                            std::string solution = solver.Solve_Cube(copy, 8);
          
                            if (!solution.empty()) {
          
                                enqueueMoveString(solution);
          
                                std::cout << "Solution: " << solution << std::endl;
          
                            } else {
          
                                std::cout << "No solution found within depth limit" << std::endl;
          
                            }

Copilot uses AI. Check for mistakes.

Visual Engine/src/CubeRenderer.cpp

Comment on lines +482 to +487

+                          const char* moves[] = {"R","R'","L","L'","U","U'","D","D'","F","F'","B","B'"};
+                          std::string scramble;
+                          for (int i = 0; i < 5; i++) {
+                              if (i > 0) scramble += " ";
+                              scramble += moves[rand() % 12];
+                          }

Copilot AI Mar 31, 2026

rand() is used here without including <cstdlib>, which can fail to compile on some toolchains. Add the proper include (and consider switching to <random> for better-quality scrambling and to avoid global RNG state).

Copilot uses AI. Check for mistakes.

Owner Author

ShahriarAhnaf Mar 31, 2026

@copilot apply changes based on this feedback

Copilot AI Mar 31, 2026

Applied in commit 5d1a16f. Added #include <random> and replaced rand() % 12 with a static std::mt19937 seeded from std::random_device and std::uniform_int_distribution<int>(0, 11) for better-quality scrambling without global RNG state.

Visual Engine/src/CubeRenderer.cpp Outdated Show resolved Hide resolved

Visual Engine/src/CubeRenderer.cpp

Comment on lines +521 to +526

    
                  GLuint vert = createShader(GL_VERTEX_SHADER, vertexCode);

                  GLuint frag = createShader(GL_FRAGMENT_SHADER, fragmentCode);

                  // Create shader program

                  shaderProgram = glCreateProgram();

                  glAttachShader(shaderProgram, vertexShader);

                  glAttachShader(shaderProgram, fragmentShader);

                  glAttachShader(shaderProgram, vert);

                  glAttachShader(shaderProgram, frag);

Copilot AI Mar 31, 2026

compileShaders() doesn't handle shader compilation failure robustly: createShader() can return 0, but the code still attaches and links, which can produce confusing errors and leak a program object. Add checks for vert/frag == 0 and early-return (with cleanup) before creating/attaching the program.

Copilot uses AI. Check for mistakes.

Owner Author

ShahriarAhnaf Mar 31, 2026

@copilot apply changes based on this feedback

Visual Engine/src/CubeRenderer.cpp Show resolved Hide resolved

Core/src/Solver.cpp Show resolved Hide resolved

Core/src/Cube.cpp Show resolved Hide resolved

Core/Inc/Solver.h Outdated

Comment on lines +108 to +109

		if (cur_group == prev_group) return true;
		if (cur_group % 2 == 0 && prev_group == cur_group + 1) return true;

Copilot AI Mar 31, 2026

is_redundant_move_idx()'s "opposite pairs" pruning is asymmetric: it prunes when cur_group is even and prev_group == cur_group + 1, but not the reverse (e.g., it prunes L after R, but not R after L). Either make the check symmetric (e.g., abs(cur_group - prev_group) == 1 && min(cur_group, prev_group) % 2 == 0) or adjust the comment so it matches the actual pruning behavior.

Suggested change

      
                    if (cur_group == prev_group) return true;
          
                    if (cur_group % 2 == 0 && prev_group == cur_group + 1) return true;
          
                    // Same face group: sequences like L followed by L' or L2 are redundant
          
                    if (cur_group == prev_group) return true;
          
                    // Opposite face groups: prune both directions (e.g., L after R and R after L)
          
                    int diff = cur_group - prev_group;
          
                    if (diff < 0) diff = -diff;
          
                    int min_group = (cur_group < prev_group) ? cur_group : prev_group;
          
                    if (diff == 1 && (min_group % 2 == 0)) return true;

Copilot uses AI. Check for mistakes.

CMakeLists.txt

Comment on lines +8 to +15

+              if(NOT CMAKE_BUILD_TYPE)
+                  set(CMAKE_BUILD_TYPE Release)
+              endif()
+              set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra")
+              set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -g")
+              set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -O3")

Copilot AI Mar 31, 2026

Global CMAKE_CXX_FLAGS* modification applies to all targets (including fetched dependencies) and can cause unexpected build issues across platforms/toolchains. Prefer setting warning/optimization flags per-target via target_compile_options() (and generator expressions for Debug/Release) on CubeCore, RubiksCubeVisualizer, and Test_Cube.

Copilot uses AI. Check for mistakes.

Copilot started work on behalf of ShahriarAhnaf

March 31, 2026 21:03

ShahriarAhnaf and others added 4 commits

March 31, 2026 14:04


          Update Visual Engine/src/CubeRenderer.cpp

e8a630b

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>


          Update Core/Inc/Solver.h

895e7e0

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>


          Update Visual Engine/src/CubeRenderer.cpp

1f6c7e2

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>


          Replace rand() with <random> for better-quality scrambling

5d1a16f

Agent-Logs-Url: https://github.com/ShahriarAhnaf/CubeSolver/sessions/ed8450c6-58cc-4f40-ade2-7cc0fdd2c1f6

Co-authored-by: ShahriarAhnaf <70302254+ShahriarAhnaf@users.noreply.github.com>

Copilot finished work on behalf of ShahriarAhnaf

March 31, 2026 21:06

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet