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

Implement permissions #59

Merged
merged 6 commits into from
May 10, 2020
Merged

Implement permissions #59

merged 6 commits into from
May 10, 2020

Conversation

wgpierce
Copy link
Collaborator

@wgpierce wgpierce commented May 9, 2020

Implement the rest of permissions and hook them up to the CLI and eventually GUI.
This brings us to feature parity with the original SAM!

Implement achievement permissions and hook up both achievement
and stats permission to the CLI.

Need to decide how to hook them up in the GUI.
@wgpierce wgpierce changed the base branch from master to dev May 9, 2020 06:58
@wgpierce
Copy link
Collaborator Author

wgpierce commented May 9, 2020

@PaulCombal how do you want to show protected stats/achievements in the GUI? SAM showed them as a red background, but because theming, I'm not so sure if that's what we want to do. If you want to implement it yourself now too, feel free.

@PaulCombal
Copy link
Owner

How do you feel about an icon with a lock just like the diamond for "rare" or the chart for "next most achived", and disabling the toggle button on the right?

Also nice work! I really have to think of a way to make the code cleaner imo asap, since I think there will soon be way enough for a release.

Make KeyValue::get/get2 always return a node, and check the returned
node's validty to check errors instead of NULL check. It is not an error
for certain fields to not be present.
Fix 4-space indending.
Remove unused functions.
@wgpierce
Copy link
Collaborator Author

Done:
image

I don't really want to restrict the user from trying to change it in case maybe they find out something can be changed! But there's a big warning sign now.
I put a few cleanups in there too.
I'm ready to merge this now.

@PaulCombal
Copy link
Owner

Alright, feel free to merge when you're ready. Did you have time to look for the achievement permission as well (issue #53) ?

@wgpierce
Copy link
Collaborator Author

Yeah achievement permissions are implemented too.

@wgpierce wgpierce merged commit 27de0b9 into dev May 10, 2020
@wgpierce wgpierce deleted the feat-permissions branch May 17, 2020 03:35
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.

2 participants