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

Options System using PlayerPrefs #71

Merged
merged 29 commits into from
Jun 18, 2017
Merged

Options System using PlayerPrefs #71

merged 29 commits into from
Jun 18, 2017

Conversation

DracoGideon
Copy link

@DracoGideon DracoGideon commented Feb 2, 2017

Implements #48.
An attempt on reflection-based Option System.

  1. Data are stored persistently using PlayerPrefs.
  2. Has a Dictionary<Type, object> that holds all Option objects
  3. Not sure if this is type-safe. There are conversions that do not get caught by compiler.

public class AudioOptions
{
[Option("Master")]
public float master { get; set; }
Copy link
Member

Choose a reason for hiding this comment

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

Upper case public properties.

[Option("SFX")]
public float sfx { get; set; }

public void Print()
Copy link
Member

Choose a reason for hiding this comment

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

Either use Log.Debug as it is functionally more complete than Debug.Log.

Initialize();
GetDataFromPrefs();
var audio = Get<AudioOptions>();
audio.Print();
Copy link
Member

Choose a reason for hiding this comment

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

These value initialization should go in the individual Options classes themselves.

public class OptionCategory : Attribute {
private string name;
public OptionCategory(string name)
{
Copy link
Member

Choose a reason for hiding this comment

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

Use K&R style bracing. We may want to set up a style-guide for Hourai Teahouse as a whole.

select t;

StringBuilder allPropertiesStr = new StringBuilder();
foreach (var t in q)
Copy link
Member

Choose a reason for hiding this comment

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

Do not use single letter variable names in foreach loops.


T Get<T>()
{
object obj = optionObjs[typeof(T)];
Copy link
Member

Choose a reason for hiding this comment

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

Use TryGetValue here instead.


foreach (var p in t.GetProperties())
{
StringBuilder keyStr = new StringBuilder();
Copy link
Member

Choose a reason for hiding this comment

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

For short strings, use string interpolation via string.Format or just normal concatenation. StringBuilder is more suited for long complex strings.

@james7132 james7132 changed the title A quick and sloppy implementation of Option System using PlayerPrefs Options System using PlayerPrefs Feb 5, 2017
@james7132 james7132 added this to In Progress in Perpetual Roadmap Mar 23, 2017
@james7132 james7132 removed this from In Progress in Perpetual Roadmap May 26, 2017
@james7132 james7132 self-assigned this Jun 13, 2017
@james7132
Copy link
Member

This is mostly ready, mainly missing two things:

  • UI integration with the rest of the game
  • An event based listener for listening to changes to values.

@james7132 james7132 merged commit 5eeb060 into develop Jun 18, 2017
@james7132 james7132 deleted the OptionSystem branch July 24, 2017 04:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants