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

Do not crash with malformed visitors.json files #35

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

marado
Copy link
Contributor

@marado marado commented Oct 3, 2020

If the visitors.json file does not contain a valid JSON file, reset
it, and act as if it was empty.

This crash was mentioned initially in #34 .

If the visitors.json file does not contain a valid JSON file, reset
it, and act as if it was empty.
@Lucidiot
Copy link

Lucidiot commented Oct 3, 2020

Doesn't this imply someone could just kill someone else's plant by making the JSON file invalid if their owner did not visit in 5 days, since the game will act as if nobody visited at all? the visit option is made to allow others to babysit your plant if you're away for more than 5 days so that would be a problem.

What about 'just' crashing but with a readable error message, or just preventing the death detection from triggering in this case and giving the owner a last chance?

@marado
Copy link
Contributor Author

marado commented Oct 18, 2020

Well, I think it is all a question of... design, more than implementation.
visitors.json is writable by anyone, and so anyone can write invalid json in there, either on purpose or by mistake. We can deal with an invalid json in two different ways:

  • assume the worse, weather or not you had visitors before, the latest visitor you had ruined things for your plant, and it got as bad as if it had never been irrigated before you went there;
  • assume the best, and just consider the plan very well irrigated.

I prefer the first one, just because (a) relying on others to water your plants is always a risk, and (b) if we "assume the best" this is also a cheating mechanism (just mess up with the visitor.json file to make sure your plan never dies).

I don't have strong feelings about this matter, I just did what seemed to make most sense to me. If we want a different behavior than the one currently implemented (eg, preventing death detection on this case), I can make the needed changes, but I suppose a decision should be made before I throw more code into the PR.

@jmdejong
Copy link
Contributor

I think the best would be not to take any automatic action but leave it up to the user.

Maybe it would be best to make a prompt if visitors.json is malformed: the user can choose to either have it deleted automatically, or to try to fix it manually (which will close botany while printing instructions on how the file should look)

@marado marado requested a review from jifunks February 26, 2021 22:59
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.

4 participants