Skip to content
This repository has been archived by the owner on Jan 1, 2019. It is now read-only.

Add custom sets, fix a few bugs #131

Merged
merged 5 commits into from
Oct 20, 2015
Merged

Add custom sets, fix a few bugs #131

merged 5 commits into from
Oct 20, 2015

Conversation

tjhorner
Copy link
Contributor

This pull request adds integration with cahcreator.com, a lightweight, collaborative Cards Against Humanity deck creator. Game hosts will be able to add custom decks to their games via the deck ID on CAH Creator; they will not be permanently stored on the server. However, decks must have at least 3 black cards and 5 white cards. They do count as expansions, so users will still need to pick a main deck.

I don't really know Knockout.js that well, so if you find anything that could be improved with it, please tell me! I added some TODOs where I believe that the code could be improved or a feature could be added.

I'm probably going to be adding raw JSON import if you don't merge this right away -- so if you want to use that instead of some shady third-party service, then hold off from merging it until then. 😉 (CAH Creator is actually open-source...)

Oh, and don't mind the whitespace removal on the migrations, it should't affect them. Atom likes saving bytes.

Things that need to be fixed or added

  • Auto adding sets (as in, a button on CAH Creator to add to your current game)
  • Add the custom set to the expansions once it's added and disable the modal (right now, users get notified through chat that the host added a custom set)

(when I fixed the merge conflict, it apparently still had the commits from master. they don't seem to be a problem but I can close and re-open this again with them removed if you want, I was just too lazy and it's 11:30 PM here)

@Rylius
Copy link
Owner

Rylius commented Oct 20, 2015

Ultimately (version 2.0 or something) I'd like to abstract deck loading quite a bit more so we could register "deck providers" without having to touch the actual game class. This PR is fine for now, moving the changes later shouldn't be too much trouble. Just a FYI for what I have planned :)

What bothers me is the additional JSON route, I'd prefer to have the custom set stuff inside the general lobby updates.
Using the existing lobby updates it's also much easier to add the custom sets to the Knockout view model, so the set can be added to the existing expansions tab.
Should also make it easier to handle multiple custom sets instead of just one, as well as removing them again (which doesn't seem possible right now?).
That'd mean moving the CAH Creator API call into Game#update... not a good place, but this can be refactored into loading decks asynchronously as they are added later. I like that. :)

I can make those changes myself if that's cool with you.

Is there any specific reason why there need to be at least 3 white and 5 black cards? CAE doesn't really care about the number of cards in a deck; empty decks are perfectly fine (even if they make little sense).

@Rylius
Copy link
Owner

Rylius commented Oct 20, 2015

Great work on CAH Creator by they way, highly appreciate the work!
Lemme know if you need any additional API routes for tighter integration (automatically adding the deck to a game would be sweet as hell!).

@tjhorner
Copy link
Contributor Author

Is there any specific reason why there need to be at least 3 white and 5 black cards? CAE doesn't really care about the number of cards in a deck; empty decks are perfectly fine (even if they make little sense).

You just answered your own question haha, an empty deck would just make no sense. I can remove the check if you'd like.

@tjhorner
Copy link
Contributor Author

as well as removing them again (which doesn't seem possible right now?).

Well, it uh, almost is. You could refresh the page, make an empty deck on CAH Creator and then use that but no, there really isn't a way to remove Game.customSet after it's been assigned a set.

Ultimately (version 2.0 or something) I'd like to abstract deck loading quite a bit more so we could register "deck providers" without having to touch the actual game class

That'd be pretty sweet!

I can make those changes myself if that's cool with you.

Go for it, but that's the entire reason why I didn't put it in /game/update heh

Rylius added a commit that referenced this pull request Oct 20, 2015
Add custom sets, fix a few bugs
@Rylius Rylius merged commit b6222d6 into Rylius:development Oct 20, 2015
@Rylius
Copy link
Owner

Rylius commented Oct 20, 2015

Gonna be making the mentioned changes now-ish

@tjhorner
Copy link
Contributor Author

Sounds good, approximately when will this be merged into master and deployed to prod?

@Rylius
Copy link
Owner

Rylius commented Oct 20, 2015

I have no idea!

If everything goes well in a couple hours, we'll see

@Rylius
Copy link
Owner

Rylius commented Oct 20, 2015

342f428

Added support for multiple custom decks, added a list of currently selected custom decks (shown to all players in the lobby) and added ability to remove custom decks.
Wanna go live? :)

@tjhorner
Copy link
Contributor Author

Yes. I'm late, it's already live!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants