-
Notifications
You must be signed in to change notification settings - Fork 11
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
Main menu/ Select level/ Options/ New Level/ Progression/ Jumpchain/ GameOver #9
base: master
Are you sure you want to change the base?
Conversation
…le other change Update Unity version to 2017.4.4f1: - Fix regressions and obsoletes code Main menu feature : - Add new scene (MainMenu) and new script (MainMenu.cs, OptionsMenu.cs) - Add select menu in in same scene (PlayMenu) - Add options menu in same scene (OptionsMenu). Change options use SettingManager.cs and GameSettings.cs to save and load settings in a json file : "gameSettings.json" New level feature : - Add new scene (Level2) and modify script - Script theme forest with new sprites created by @Sihg87 and used with her permission PauseMenu - Add PauseMenu in Level1 and Level 2 - Change Pauser.cs script to implemant the PauseMenu Other change : - Now the player can chain several jumps (actualy it's define to 2 jump in PlayerControl.cs) - Prevents actions during when the pause it's enable - Prevents the player from leaving the level boundry (Add insibleWall) -Gives the possibility to configure the starting rotation of the enemy (left, right or random, this is configurable on spawners) ... WARNING, this commit it's not the final version.
Fix many problemes witth gameSettings - Add a progression : scoreLock with link between level, reach an amount of score to unlock levelX
- Now the options modify in the mainMenu is apply in the differents levels. The music and the different effects has been modify to take the sound option. - Add StatsService and SettingsService (static class) to manage score and options more easly - Other fix - Optimize code and add many comments
- Add comment - Fix GameOver - Fix Score -Fix Obsolete code (score, scoreshadow et bomb)
.gitignore
Outdated
unitydemo-2DPlatformer.csproj | ||
|
||
*.unitydemo-2DPlatformer.csproj |
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.
Something that we probably should have done here is to add the items from here.
https://github.com/github/gitignore/blob/master/Unity.gitignore
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.
Indeed, it's a much cleaner version than mine.
Since I'm going to make a new commit. Let me to add this version.
Assets/Scripts/GameStats.cs
Outdated
|
||
public class GameStats | ||
{ | ||
private string nameSettingFile = "gameStats.json"; // Name of the stats file. |
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.
We will stick to tabs in our projects, we should be careful to check the format of the files when making a PR
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.
Oh! I used the basic configuration of Visual Studio, I will change that immediately. Sorry, it must have been horrible to read.
Assets/Scripts/Gun.cs
Outdated
{ | ||
// ... set the animator Shoot trigger parameter and play the audioclip. | ||
anim.SetTrigger("Shoot"); | ||
audio.Play(); | ||
GetComponent<AudioSource>().volume = SettingsService.GetVolumeEffect(); |
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.
This is probably because of the update to the new Unity, but, how do you think this could be made more efficient?
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.
I think it would be better to store the "GetComponent ()" in the Awake part. Which avoids calling the GetComponent () every time. I correct that too.
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.
The same error was made in MusicPlayer.cs, so i correct that. Thanks.
Assets/Scripts/GameStats.cs
Outdated
{ | ||
private string nameSettingFile = "gameStats.json"; // Name of the stats file. | ||
|
||
public List<LevelStats> levelStats; // List of level stats of each level. |
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.
Are there any other data structures you considered for this, and if so, why did you discount them?
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.
To be honest, this is the first structure that came to mind. Using this structure daily in my development, I am more comfortable with it. But I think that other structures such as a dictionary or a hashtable could be more efficient in this case.
Assets/Scripts/Pauser.cs
Outdated
// Use this for initialization | ||
void Start() | ||
{ | ||
PauseMenuObject.SetActive(IsPaused); |
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.
Is this necessary?
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.
No indeed. This same code "PauseMenuObject.SetActive(IsPaused)" is in the Resume() method ... Called just after this line. I will correct that.
However the Resume() method in Start() is necessary to ensure the game is no longer paused. (When we enter in a level, we go to the menu and we return to the same level)
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.
Another mistake on my part is to have used the variable "isPaused" here while I have to use the one from Settings Service. I correct that too.
Hi, did a quick review and left a few questions. Your responses and opinions on these topics would be appreciated! |
- Get the community version of .gitignore - Change the isPaused in Pauser.cs and change the use of this variable in another class (now we use SettingsService.isPaused) - Optimization of the application of the volume of the gun and the music - Fix the tabulation
Hello ! Thank you for your return, I applied some of your tips, especially the one about tabs and made a new commit (this should make the code easier to read) |
Pull request for implemented #3 issue and other feature (Jump chain, Options and progression)