Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Map/Level Select Functionality #40

Merged
merged 2 commits into from
Mar 24, 2024
Merged

Conversation

MoldyPotat
Copy link
Contributor

Overview

This added a map/level select functionality to allow user to choose what level they want to play.

Related Issue(s)

Closes Jira LQ-56

Changes

Only preexisting file modification is to game_scene script where I changed play button routing to map scene instead of game scene.

Added

  • Added Scripts folder and MapScene sub folder to start organizing scripts.
  • Added Level1.gd this adds the functionality to the level buttons allowing for the confirmation popup and changing scenes
  • Added MapButton.tres this a button group to allow better functionality of level buttons
  • Added map_scene this is where all the additions are located and where the map/level select is.

Changed

Only preexisting file modification is to game_scene script where I changed play button routing to map scene instead of game scene.

Removed

Nothings was removed.

Fixed

No bug fixes

Screenshots

image

Instructions for Testing

Provide detailed instructions on how reviewers can test the changes. Include steps to follow, any relevant URLs, user credentials if needed, and expected outcomes.
No Testing has been added yet will be added in future. Currently tested manualy by running the game and testing.

Checklist before Merging

  • I have performed a self-review of my own code.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added necessary documentation (if appropriate).
  • I have ensured the changes adhere to the project's coding standards and guidelines.

Additional Comments

Include any additional comments or context that you think might be helpful for the reviewers.

Created a level select scene with popout menus for confirmation. Also added a scripts folder to better organize scripts.
Copy link
Contributor

@barkangel barkangel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Review checklist

  • Readability:

    • Code in gd script was easy to understand.
    • Functions have simple and easy to understand names.
  • Code Quality:

    • Code is clean and comments are applicable.
  • Testing:

    • Github Actions CI system passed all tests.
    • Test coverage is not high, but that is OK for now.
    • Manual testing instructions listed.
  • Functionality:

    • Yes, changes backward compatible. PR resolves existing issue JIRA LQ-56
  • Additional Comments:

    • Good screenshot that helps visualize changes.
    • No documentation in map_scene.tscn but I believe it's the map screen so it's ok for now.

Copy link
Contributor

@dusek2 dusek2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Review:

  • Readability: Code readable and easy to understand

  • Code Quality: Code quality confirmed.

  • Testing: We need to implement coverage test into CI workflow

  • Functionality: Code seems functional

  • Housekeeping: No extra files or bloat

Additional Comments:

@thegreatzoidberg
Copy link
Contributor

Pull Request Review:

  • Readability: map_scene.tscn could use some comments but the rest of the files are good.

  • Code Quality: Code is well spaced-out and looks clean.

  • Testing: No testing files were added but the CI workflow is broken right now.

  • Functionality: Code looks functional to me.

  • Housekeeping: No extra logs or text files. No AI was mentioned.

Additional Comments:

  • Nice screenshot. Helps illustrate what this code does.

@MoldyPotat MoldyPotat merged commit a564aa0 into UNLV-CS472-672:main Mar 24, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants