Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add Faker::Games::DungeonsAndDragons #1794

Closed
wants to merge 26 commits into from

Conversation

jameslvdb
Copy link

@jameslvdb jameslvdb commented Oct 5, 2019

Created Faker::Games::DungeonsAndDragons generator. It currently supports the following methods:

  • ability: returns one of the six standard D&D abilities.
  • skill: returns a random skill. If passed one of the abilities from above, it will return a skill corresponding to that ability.
  • alignment: returns one of the nine D&D alignments.
  • armor: returns the name of a piece of armor.
  • player_class: returns the name of one of D&D's core classes (used player_class to avoid conflict with the class reserved word
  • subclass: returns a random subclass. If passed one of the classes from above, it will return a subclass corresponding to that class.
  • language: returns the name of one of the languages from the D&D universe.
  • monster: returns the name of a monster from D&D.
  • race: returns the name of a race from D&D.
  • subrace: returns a random subrace. If passed one of the races from above, it will return a corresponding subrace.
  • spell: returns the name of a spell.
  • magic_school: returns the name of one of the schools of magic.
  • weapon: returns the name of a weapon.
  • magic_item returns the name of a magic item.

This is a large generator (41K) -- I went a little all-out. If any maintainers would like this generator to be smaller before approving this PR, I'll be happy to make some updates. Even if the size is okay, I'm a little on the fence about keeping the language, weapon, and armor methods around. While these are important to D&D, they are likely served just fine by other generators for the purposes of testing, and they're small enough collections that if one does feel strongly about having an accurate generator, adding them locally on a per-project basis wouldn't be difficult.

Add basics tests for armor, player_class, monster, race, spell, and
weapon collections.
Added Faker::Games::DungeonsAndDragons.subclass. Takes an optional
parameter player_class. If passed a valid parameter, it will return the
corresponding subclass. If passed an invalid parameter, it will throw an
ArgumentError. If not passed a parameter, it will return a random
subclass.
Behaves similarly to the subclass method: takes an optional parameter:
returns the associated subrace if the parameter is valid, and raises an
ArgumentError if invalid. Returns a random subrace if no parameter is
given.
'Artificer' and 'Goliath' are invalid options in the scope of what the
current options are, but are actual options created by Wizards of the
Coast in content not covered by the System Reference Document. These
were changed to 'Jedi' and 'Wookiee' to make it obvious that they are
invalid options.
Copy link
Contributor

@Zeragamba Zeragamba left a comment

Choose a reason for hiding this comment

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

you passed your programming check. this time

@Zeragamba Zeragamba self-assigned this May 23, 2020
Copy link
Contributor

@Zeragamba Zeragamba left a comment

Choose a reason for hiding this comment

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

Oh, hold up. We failed our perception checks... Faker::Games:DnD already exists. Can we merge all of this into there?

@jameslvdb
Copy link
Author

jameslvdb commented May 23, 2020

@Zeragamba I think there would be some duplication here if we merge as is. The previous PR refers to races as 'species', while this one uses 'races' (which reflects the terminology in the Player's Handbook). I'd be happy to pull down again and make updates to make it all line up!

Never updated an open PR where new changes conflict with mine before, so it might take a little while but I'll take a look 😄

@Zeragamba
Copy link
Contributor

Merge faker/master into your local branch and you should be good to go :)

@dollerbill
Copy link
Contributor

@jameslvdb I did a little bit of cleanup with #2098 which might make a refactor of this PR a bit easier. I reintroduced races as well as added language and monster. I added weapons but broke them down between melee and ranged. It looks like everything else covered with your additions shouldn't have any conflicts, so could be a pretty easy merge

Refactored the existing DnD class to include my content from the
DungeonsAndDragons class/locale. Also added .tool-versions to the
.gitignore so that asdf files won't be included on accident.
@jameslvdb
Copy link
Author

Thanks @dollerbill! Honestly forgot about this PR, thanks for reminding me 😆

Since the DnD class has already been introduced, I went ahead and moved all of my additions into DnD and removed all of the DungeonsAndDragons-related code.

I also added .tool-versions to the .gitignore file for the asdf users who might contribute in the future 😄

@Zeragamba is this what you were looking for?

README.md Outdated
@@ -235,6 +235,7 @@ gem 'faker', :git => 'https://github.com/faker-ruby/faker.git', :branch => 'mast
- [Faker::Game](doc/games/game.md)
- [Faker::Games::DnD](doc/games/dnd.md)
- [Faker::Games::Dota](doc/games/dota.md)
- [Faker::Games::DungeonsAndDragons](doc/games/dungeons_and_dragons.md)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- [Faker::Games::DungeonsAndDragons](doc/games/dungeons_and_dragons.md)

looks like this line might be a leftover from the original PR

@dollerbill
Copy link
Contributor

@jameslvdb I didn't mean you had to do it right this second 😂
but your additions look great, this is probably as complete as a DND generator could possibly be!

@stefannibrasil
Copy link
Contributor

Hey there. In an effort to lighten our load as maintainers and be able to serve you better in the future, the faker-ruby team is working on cleaning out the cobwebs in this repo by pruning the backlog. As there are few of us, there are a lot of items that will simply never earn our attention in a reasonable time frame, and rather than giving you an empty promise, we think it makes more sense to focus on more recent issues. That means, unfortunately, that we must close this PR.

Don't take this the wrong way: our aim is not to diminish the effort people have made or dismiss problems that have been raised.

If you feel that we should reopen this PR, please do the following first:

  • rebase with master
  • verify this contribution is still relevant (i.e. hasn't been implemented/solved yet)
  • run tests again

Then please let us know so that we can re-prioritize it. Thanks!

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants