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

Added clearSave function to Data class. #142

Merged
merged 3 commits into from Nov 22, 2014
Merged

Conversation

mikeevmm
Copy link
Contributor

@mikeevmm mikeevmm commented Jul 2, 2014

This would be particularly useful either for resetting the game or to erase wrongly saved values.

@jacobalbano
Copy link
Contributor

I'd personally call it clear() instead of clearSave(), for consistency.

Also, being as I'm unfamiliar with the inner workings of this class, why does the new function clear _shared instead of _data? It seems to use _data everywhere else. Never mind, now realizing that one is a SharedObject and one is an Object.

@zachwlewis
Copy link
Member

I'm a fan of clear() as well. I hope the author pops back by to answer your question.

On Fri, Nov 21, 2014 at 2:30 PM, Jacob Albano notifications@github.com
wrote:

I'd personally call it clear() instead of clearSave(), for consistency.

Also, being as I'm unfamiliar with the inner workings of this class, why does the new function clear _shared instead of _data? It seems to use _data everywhere else.

Reply to this email directly or view it on GitHub:
#142 (comment)

@mikeevmm
Copy link
Contributor Author

I'm here, but Jacob seems to have answered himself?

@AbelToy
Copy link
Contributor

AbelToy commented Nov 22, 2014

This looks fine to me, merging!

@AbelToy AbelToy closed this Nov 22, 2014
@AbelToy AbelToy reopened this Nov 22, 2014
AbelToy added a commit that referenced this pull request Nov 22, 2014
Added clearSave function to Data class.
@AbelToy AbelToy merged commit 46faa60 into useflashpunk:master Nov 22, 2014
@jacobalbano
Copy link
Contributor

The only other suggestion I have is that clear() not take a parameter. Since all of the read/write operations assume an opened SharedObject already, it feels strange to me to specify the file to clear as a parameter, especially since clear() currently calls Data.load() anyway.

@jacobalbano
Copy link
Contributor

@Rolpege I was in the middle of writing a comment when you merged, haha. Could you take a look at my suggestion if you missed it?

@AbelToy
Copy link
Contributor

AbelToy commented Nov 22, 2014

My fault! Yes, I agree. clear() should probably be only _shared.clear().

Also, maybe flushing after clearing would be a good idea?

@jacobalbano
Copy link
Contributor

I'm torn about that...it seems like the logical progression, but I feel like there should be a way to clear data without also saving it. Since the save/flush step already exists in a separate function, I feel like it would be best if saving was left as its own discrete step and not implied by the clear step.

@AbelToy
Copy link
Contributor

AbelToy commented Nov 22, 2014

Yeah, probably.

So, want to add to this pull request or how do we do it? :D

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

5 participants