Better randomness when spawning cannon and target

[MrEliptik]

Put safeguards in place to make sure a minimum distance is respected between the cannon and the target

[Laozey]

Hi ! I would like to tackle this issue.

My approach would be to nest the target new position generation in a while loop. Each step it would check if the distance between the new position and the cannon position is greater than around 300 units (as the circle is 260 units wide).

Can i work on it ?

[MrEliptik]

Hey thanks for your offer! I think I would have used the same approach so you can go for it 😊

[Laozey]

https://github.com/MrEliptik/godot_twitch_games/blob/0d151e8b48984d11deb79df4d16fe0de0e20ff56/scenes/games/cannon/cannon.gd#L70-L74 Is the change_state(GAME_STATE.RUNNING) really relevant as it is already called there: https://github.com/MrEliptik/godot_twitch_games/blob/0d151e8b48984d11deb79df4d16fe0de0e20ff56/scenes/games/cannon/cannon.gd#L60-L68 change_position seems to be called only there

[Laozey]

Also I've created a local branch and commit my changes there but i have no idea how to make a pull request with it

[MrEliptik]

https://github.com/MrEliptik/godot_twitch_games/blob/0d151e8b48984d11deb79df4d16fe0de0e20ff56/scenes/games/cannon/cannon.gd#L70-L74

Is the change_state(GAME_STATE.RUNNING) really relevant as it is already called there: https://github.com/MrEliptik/godot_twitch_games/blob/0d151e8b48984d11deb79df4d16fe0de0e20ff56/scenes/games/cannon/cannon.gd#L60-L68

change_position seems to be called only there

Indeed calling change_state() again is not useful

[MrEliptik]

Also I've created a local branch and commit my changes there but i have no idea how to make a pull request with it

I personally use VSCode with Github integration for that. It works really well. You can also probably use GitHub desktop which should be easy to work with.

[Laozey]

I also use VSCode, but when i try to push my changes it asks me to set my branch up-stream and when i do so i tells me that i don't have the permission for it

[MrEliptik]

I'm not super familiar with all of that, but it might be that you didn't fork the project and by trying to set your branch up stream, it's trying to add your branch to this repo, which you can't do. See the step 1 here, is to fork the repo.

@Drathal can you confirm?

[Laozey]

As many other numerical values are hard coded, do i leave the minimum distance hard coded too or do i put it in a class variable (or even as cons or export ?)

[MrEliptik]

Put it as an export variable for now. We'll do a cleaning round later.