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 support for a custom advancement on first time passing an exam #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

benjamin1313
Copy link
Member

added advancement feature to Exams plugin

@benjamin1313 benjamin1313 self-assigned this Feb 16, 2021
@HeroGamers HeroGamers self-requested a review February 16, 2021 22:01
Copy link
Member

@HeroGamers HeroGamers left a comment

Choose a reason for hiding this comment

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

Currently this only supports one advancement - for completing an exam.
Moreover, this one advancement requires a datapack - which is not a part of the standalone plugin.

If this would be a part of the plugin, which is kind of a standalone thing for servers, which doesn't require a datapack in and of itself, it would probably be nice if you added so there, in the exams' config file, for each exam, is an advancement that you can list, like so:

CitizenEnglish:
  RankName: citizen
  Command: give $PlayerName 38 1
  NumberOfQuestions: 10
  Advancement: doggycraft:doggycraft/citizen
  Questions:
    How do i set a home?:

And then have the plugin add that advancement to the player.

And even then, you could just do:

CitizenEnglish:
  RankName: citizen
  Commands:
    - give $PlayerName 38 1
    - advancement grant $PlayerName only doggycraft:doggycraft/citizen
  NumberOfQuestions: 10
  Questions:
    How do i set a home?:


// giving player an advancement for passing an exam if set to true
if (plugin.grantAdvancement) {
Advancement a = Bukkit.getAdvancement(NamespacedKey.fromString(plugin.examAdvacementName));
Copy link
Member

Choose a reason for hiding this comment

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

Could be changed to see if the exam has an Advancement specified.

Copy link
Member Author

@benjamin1313 benjamin1313 Feb 17, 2021

Choose a reason for hiding this comment

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

agree on the part of the if(plugin.grantAdvancement) check

Copy link
Member Author

Choose a reason for hiding this comment

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

For now one advancement works as a prof of concept.
The reason you can turn this feature on and of is exactly because people might not have a datapack on their server or want to have one. And it does not have to be a specific datapack. Thats why it can be change in the config.yml so people can create their own datapacks with the advancement they what to give.


public String serverName = "Your Server";
public String languageFilename = "english.yml";
public String examAdvacementName = "doggycraft:doggycraft/student";
Copy link
Member

Choose a reason for hiding this comment

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

This is a hardcoded advancement, for completing your first exam.
Would be better to move to the config file for the exams, to have custom per-exam advancements.

Or specify more precisely, that this advancement is a one-time advancement, by calling it something like "firstExamAdvancement".

Copy link
Member Author

Choose a reason for hiding this comment

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

agree on naming but the "hardcoded" bit is just a default value. it is change later when the config is loaded.

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.

3 participants