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

feat: in-game configuration settings #102

Merged
merged 13 commits into from
Jun 23, 2024
Merged

feat: in-game configuration settings #102

merged 13 commits into from
Jun 23, 2024

Conversation

SSelinn
Copy link
Collaborator

@SSelinn SSelinn commented Jun 19, 2024

Description

Added working in game config where you cannot go out of boundries. Also added test to verify it working.

Testability

Start in the Settings scene and press play. Play around with the values. Grid size cant be smaller than 3 or bigger than 100. The max for all other options is gridvolume *0.7 rounded up.
NOTE: items and enemies have their own max count. If you pass that it will take the max allowed number and divide it by 3 or 2 respectively.

NOTE: sorry for the wrong branch name. It served as a backup if I was not able to merge correctly with main due to all the prefab conflicts, but I managed!

Checklist

  • Set the proper pull request name
  • Merged main into your branch
  • Added a scene to the game for your changes

Copy link
Contributor

@SilasPeters SilasPeters left a comment

Choose a reason for hiding this comment

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

You're code quality is getting better! Nice way of dividing things into methods.

aplib.net-demo/Assets/Scripts/Configuration.cs Outdated Show resolved Hide resolved
aplib.net-demo/Assets/Scripts/Configuration.cs Outdated Show resolved Hide resolved
aplib.net-demo/Assets/Scripts/Configuration.cs Outdated Show resolved Hide resolved
Copy link
Contributor

@JoenvandeVorle JoenvandeVorle left a comment

Choose a reason for hiding this comment

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

Nice work overall! I'm sorry for the many comments :/

aplib.net-demo/Assets/Scripts/Configuration.cs Outdated Show resolved Hide resolved
aplib.net-demo/Assets/Scripts/Configuration.cs Outdated Show resolved Hide resolved
aplib.net-demo/Assets/Scripts/Configuration.cs Outdated Show resolved Hide resolved
aplib.net-demo/Assets/Scripts/Configuration.cs Outdated Show resolved Hide resolved
aplib.net-demo/Assets/Scripts/Configuration.cs Outdated Show resolved Hide resolved
aplib.net-demo/Assets/Scripts/Configuration.cs Outdated Show resolved Hide resolved
aplib.net-demo/Assets/Scripts/Configuration.cs Outdated Show resolved Hide resolved
@QuakeEye
Copy link
Contributor

You're code quality is getting better! Nice way of dividing things into methods.

Your*

SilasPeters
SilasPeters previously approved these changes Jun 21, 2024
@QuakeEye QuakeEye removed the request for review from JoenvandeVorle June 22, 2024 18:58
@Tboefijn Tboefijn requested review from JoenvandeVorle and removed request for JoenvandeVorle June 23, 2024 15:06
@Tboefijn Tboefijn enabled auto-merge (squash) June 23, 2024 22:32
@Tboefijn Tboefijn requested a review from SilasPeters June 23, 2024 22:32
JensSteenmetz
JensSteenmetz previously approved these changes Jun 23, 2024
Copy link
Member

@JensSteenmetz JensSteenmetz left a comment

Choose a reason for hiding this comment

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

LGTM!

@Tboefijn Tboefijn disabled auto-merge June 23, 2024 23:05
@Tboefijn Tboefijn enabled auto-merge (squash) June 23, 2024 23:05
@Tboefijn Tboefijn requested a review from JensSteenmetz June 23, 2024 23:05
Copy link
Member

@JensSteenmetz JensSteenmetz left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link

Copy link
Contributor

@SilasPeters SilasPeters left a comment

Choose a reason for hiding this comment

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

Small suggestion. It's a social experiment whether you will fix it

aplib.net-demo/Assets/Scripts/Configuration.cs Outdated Show resolved Hide resolved
@Tboefijn Tboefijn disabled auto-merge June 23, 2024 23:20
@Tboefijn Tboefijn enabled auto-merge (squash) June 23, 2024 23:20
@Tboefijn Tboefijn disabled auto-merge June 23, 2024 23:20
@Tboefijn Tboefijn self-requested a review June 23, 2024 23:21
@Tboefijn Tboefijn enabled auto-merge (squash) June 23, 2024 23:23
@Tboefijn Tboefijn merged commit e83b68a into main Jun 23, 2024
3 checks passed
@Tboefijn Tboefijn deleted the feat/167-config branch June 23, 2024 23:24
Copy link

🎉 This PR is included in version 3.34.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants