-
Notifications
You must be signed in to change notification settings - Fork 32
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: New Player system spawn options #384
Conversation
Reviewer's Guide by SourceryThis pull request introduces new player spawn options, allowing players to select their home and recruiting world locations. It also includes a radio button constructor to simplify the creation of these options. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @OH296 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding documentation comments explaining the spawn location selection algorithm in scr_system_spawn_functions.gml, particularly around the fringe vs central spawn logic
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
40bd382
to
abbbfe4
Compare
f4fa111
to
3f52882
Compare
511ae7e
to
f578627
Compare
@sourcery-ai review |
@sourcery-ai review |
@sourcery-ai review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @OH296 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Please add additional validation to ensure player spawn locations are valid and not too close to enemy factions
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Description of changes
Adds new options to player home and recruit options
adds a map location spawn consisting of fringe or central for players that wish to control the location of their spawn star
adds three options for recruitment and home star spawn relations (should be self explanatory)
created a radio_buttons constructor to make this easier
removed a bug where two systems could spawn with the players home name
removed a bug with
Reasons for changes
Related links
How have you tested your changes?
Summary by Sourcery
Add new player spawn options for homeworld and recruiting world locations. Allow players to choose between fringe or central spawn locations and configure the relationship between homeworld and recruiting world (same planet, same system, or different systems).
New Features: