-
Notifications
You must be signed in to change notification settings - Fork 36
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
Add Main Menu Sample project. (Broken) #4
base: master
Are you sure you want to change the base?
Conversation
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.
Okay, I took a look at this bad boi! Here are the things that stood out the most
- Are the CSG files (
CSG_Collision.flax
and more) actually used or are they leftovers? - I haven't managed to dig up the Sponza license
- What's
ReplaceWithYourMusic.flax
?
|
||
ReadSettingsFile(); | ||
|
||
string lastRead_Formed = ""; |
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 can't really encourage the use of this form of parser. Seems pretty brittle and hack-ish
if (FullscreenStatus) | ||
{ | ||
_retreivedSettings = _retreivedSettings + Environment.NewLine + FullscreenPreset + " = " + SettingsFileTrueTerm + ";"; | ||
}else |
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.
else
statements go on their own line, but that's mostly formatting nitpicking
_writtenSettingsValue = _retreivedSettings; | ||
} | ||
|
||
File.WriteAllText(_dataPath, _writtenSettingsValue); |
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 perfect settings setup (see: whatever I posted on Discord) would do this asynchronously instead of potentially blocking things.
{ | ||
if (!_optionsMenuActive) | ||
{ | ||
FullscreenButton.IsActive = true; |
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.
Wouldn't it be easier to put all those buttons into a panel and enable/disable that panel?
@@ -0,0 +1,10 @@ | |||
using FlaxEngine; | |||
|
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.
Missing namespace
@@ -0,0 +1,105 @@ | |||
using FlaxEngine; | |||
|
|||
namespace BasicTemplate |
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.
Wrong namespace, I'm guessing it's a typical "copy pasta" mistake.
At least pasta is tasty 🍝
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 really forgot that one.
if (lastRead_Formed.Contains(GFX_Preset + " = " + SettingsFileTrueTerm + ";")) | ||
{ | ||
gfxBool = true; | ||
}else | ||
{ | ||
gfxBool = false; | ||
} | ||
|
||
bool fullscreenBool; | ||
if (lastRead_Formed.Contains(FullscreenPreset + " = " + SettingsFileTrueTerm + ";")) | ||
{ | ||
fullscreenBool = true; | ||
}else | ||
{ | ||
fullscreenBool = false; | ||
} | ||
|
||
bool sfxBool; | ||
if (lastRead_Formed.Contains(SFX_Preset + " = " + SettingsFileTrueTerm + ";")) | ||
{ | ||
sfxBool = true; | ||
}else | ||
{ | ||
sfxBool = false; | ||
} |
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.
Personally I find that overall there are too many string concatenations and if statements like these throughout the source, this has the potential to be simplified perhaps atleast via ternary operator or use a different implementation entirely.
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.
You're right, I should've gotten something like: bool sfxBool = lastRead_Formed.Contains(SFX_Preset + " = " + SettingsFileTrueTerm + ";")
and to have them be only 3 lines instead of 20 or so. Probably too late to change it.
No description provided.