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

House rules #5

Open
Californ1a opened this issue Mar 20, 2018 · 5 comments
Open

House rules #5

Californ1a opened this issue Mar 20, 2018 · 5 comments
Labels
house-rule Related to custom rules that are not in the game's core
Milestone

Comments

@Californ1a
Copy link
Contributor

Californ1a commented Mar 20, 2018

As per the discussion that started in #2, giving users the ability to create their own rules with the usage of events and/or options.

Copied:

I don't know if an options object is a good idea, as there's a lot of house rules existing and it could make the code very confusing, deviating from the official rules.
Maybe this could be achieved with a better use of events, and anyone that wants to add rules outside of the official ones, will implement them.

Maybe repurpose the cardplay event, so that it fires when a card is played but before it gets placed in the discard pile (and include both the card played and the current discard card in the arguments), so anyone using the lib has the ability to intercept card plays and inject their own house rules before each card gets placed. You already have the nextplayer event, which fires directly after cardplay and could include the newly placed card in the arguments, to (mostly) replace the need for the current cardplay event.

This would probably require some additional lib functionality as well so that you would have the ability to prevent certain lib actions like automatically moving to the next player or automatically drawing cards. This is where I think an options object could work well - pass in an object with the default/official rules that you want to turn off and handle manually. This would require the ability to actually do those things manually though, for example, giving the lib user the ability to draw cards into any player's hand without anything added/extra occurring like moving to the next player (i.e. if they wanted to start with 10 cards each instead of 7).

Maybe it would make sense to add player.play() and player.draw() so that the user has more control over who is playing or drawing if they want to manually do the player switching. You can also keep game.play() and game.draw() as functions to just use the default official rules, and have game.goToNextPlayer() be public, to manually move to the next player (maybe you don't want to require draw before pass).

@danguilherme
Copy link
Owner

Makes sense to add player.play() and player.draw().

Also, we don't need to stick to the existing events only, we could add before events too, like beforecardplay, and always send a game instance to the event.

Actually, I was thinking of something completely based on events. The engine itself would only know the basic, official rules. But in every key point, it would emit an event, like beforecardplay, draw, nextplayer, etc.

House rules code (that would be somehow like plugins) would attach to the necessary events and modify game flow. The engine would allow these plugins to avoid some actions, like if we return false in the beforecardplay, the engine doesn't continue with its normal flow.

The API to load different house rules could be:

Game(players[, customRules]);

As an example:

import { Game } from 'uno-engine';

import { CumulativeDrawTwo } from 'uno-engine/cumulative-draw-two'; // maintained house rule
import { CumulativeDrawWildFour } from 'uno-engine-cumulative-draw-wild-four'; // community house rule

const players = ["Player 1", "Player 2"];
const game = Game(players, [CumulativeDrawTwo, CumulativeDrawWildFour]);

I'm just throwing ideas here, open to any other solution.


Interesting links:

@Californ1a
Copy link
Contributor Author

Also, we don't need to stick to the existing events only, we could add before events too, like beforecardplay, and always send a game instance to the event.

It would definitely be a good idea to send the game instance to each event. Users might be catching the events in different files, and that would help greatly so they don't have to manually pass it around. My only concern is that the events are tied to the game instance right now, so if someone wanted to use an eventloader (like below), which is pretty typical for keeping larger projects with a lot of modules and events clean, it wouldn't be very practical because the game changes. It might make more sense to have the events on Game instead, especially if the game instance is passed into the events. Currently:

//main.js
const { Game } = require("uno-engine");
//- you would have to create the game first
let game = Game(players);
//- and then you could load the eventloader
require("./eventLoader.js")(game);
//- if you want to start a new game with different players, you have to reload the eventloader
//- not great if the eventloader is managing events for all modules in your project

//eventLoader.js
const reqEvent = (event) => require(`./events/${event}`);
module.exports = (game) => {
  //eventloader needs `game` to be able to use `game.on()`
  //you already have `game` in higher scope, and can pass it into each event yourself
  game.on("cardplay", (err, playCard, playBy) => reqEvent("cardplay")(game, err, playCard, playBy));
  game.on("nextplayer", (err, nextPlayer) => reqEvent("nextplayer")(game, err, nextPlayer));
  game.on("end", (err, winner, score) => reqEvent("end")(game, err, winner, score));
};

//events/cardplay.js
module.exports = (game, err, playCard, playBy) => {
  //cardplay event code
};

With the events being on game, you essentially already have to pass around the game instance anyway to load the events. Even if the game instance was sent back in each event, you would still have to pass the game instance around to be able to use game.on(). If the events were on Game instead, then the user wouldn't have to pass in the game instance to the eventloader, and sending the game instance in each event would make a lot more sense. As it currently is, you don't really need to send the game instance with each event because you've already got the game instance in a higher scope in order to catch the event in the first place. This would make much more sense:

//main.js
const { Game } = require("uno-engine");
require("./eventLoader.js")(Game);
let game = Game(players);

//eventLoader.js
const reqEvent = (event) => require(`./events/${event}`);
module.exports = (Game) => {
  //and if the library passed `game` instance into events
  Game.on("cardplay", (game, err, playCard, playBy) => reqEvent("cardplay")(game, err, playCard, playBy));
  Game.on("nextplayer", (game, err, nextPlayer) => reqEvent("nextplayer")(game, err, nextPlayer));
  Game.on("end", (game, err, winner, score) => reqEvent("end")(game, err, winner, score));
};

//events/cardplay.js
module.exports = (game, err, playCard, playBy) => {
  //cardplay event code
};

Game doesn't change so you never have to reload the eventloader itself. This would also be a step toward allowing multiple games to run at the same time, with events for any of the running games triggering Game.on(). Multiple games at the same time is almost essential to what I'm trying to do with a chat bot that may be running on various servers and channels, where each server could have their own uno chat channel. It would be really strange if only one server could play at a time.


import { Game } from 'uno-engine';

import { CumulativeDrawTwo } from 'uno-engine/cumulative-draw-two'; // maintained house rule
import { CumulativeDrawWildFour } from 'uno-engine-cumulative-draw-wild-four'; // community house rule

const players = ["Player 1", "Player 2"];
const game = Game(players, [CumulativeDrawTwo, CumulativeDrawWildFour]);

That looks like a much better solution than what I had in mind. I hadn't even considered something like that. That would also allow for, as you noted, others to make their own rules plugins.

@danguilherme
Copy link
Owner

danguilherme commented Mar 23, 2018

I think this eventloader pattern is a lot confusing - never seen it. Do you have any resource about it?

Also, it doesn't make much sense to emit events from/register listeners to the constructor (factory, actually - Game) - it's like assigning click listeners to the HTMLButton object instead of in the specific <button> instance.

Events are actions that happened in one specific instance. I believe making the Game factory to dispatch events for each one of its instances is an anti-pattern.


[...] This would also be a step toward allowing multiple games to run at the same time, with events for any of the running games triggering Game.on(). [...] where each server could have their own uno chat channel. It would be really strange if only one server could play at a time.

Multiple games should not be the concern of this engine. It is responsible for managing a single Uno game. If you (aka the user) want to have a lot of games, it's up to you to manage that. You could create a factory function to create games tailored for your need:

const { Game } = require("uno-engine");
const eventLoader = require("./eventLoader.js");

function startNewGame(players) {
  let game = Game(players, [... any house rules you want as a pattern for all instances ...]);
  // if you want to use this eventloader pattern, you register it
  // or, you could create a single 'instance' of the eventloader and then register each started game in it.
  eventLoader(game);

  return game;
}

//- if you want to start a new game with different players, you have to reload the eventloader

This can be solved using the idea of the snippet above.


We can't cover every single use case in here, we should keep the engine simple, doing one thing, and doing it well: managing a single Uno gameplay.

@danguilherme danguilherme added this to the v0.1.0 milestone Mar 23, 2018
@Californ1a
Copy link
Contributor Author

Californ1a commented Mar 23, 2018

I think this eventloader pattern is a lot confusing - never seen it. Do you have any resource about it?

There isn't really any specific documentation I can find on event loaders/handlers, but it is common practice with large projects to split each event out into their own files for clarity, especially when you have many module dependencies that all have their own events. It gets cluttered in the main file otherwise. The loader itself just requires the needed event file when an event is emitted.

A more verbose eventLoader.js would look something like this

const reqEvent = function(eventName) {
  return require(`./events/${eventName}`);
};

const loadEvents = function(game) {
  game.on("cardplay", function(err, playedCard, playedBy) {
    const cardplay = reqEvent("cardplay"); //same as `require("./events/cardplay.js")`
    return cardplay(game, err, playedCard, playedBy); //injecting `game` when calling cardplay.js
  });
  game.on("nextplayer", function(err, nextPlayer) {
    const nextplayer = reqEvent("nextplayer");
    return nextplayer(game, err, nextPlayer);
  });
  game.on("end", function(err, winner, score) {
    const end = reqEvent("end");
    return end(game, err, winner, score);
  });
};

module.exports = loadEvents;

This is the same as

const reqEvent = (event) => require(`./events/${event}`);
module.exports = (game) => {
  game.on("cardplay", (err, playCard, playBy) => reqEvent("cardplay")(game, err, playCard, playBy));
  game.on("nextplayer", (err, nextPlayer) => reqEvent("nextplayer")(game, err, nextPlayer));
  game.on("end", (err, winner, score) => reqEvent("end")(game, err, winner, score));
};

This can be solved using the idea of the snippet above.

The problem with that snippet is that it assumes that the uno events are the only ones being used in the eventloader. That would work if you use a separate eventloader for each module, but the purpose of an eventloader is to load all events for all of your required modules, and dynamically require the correct file for each event when that event is called. Eventloaders have many arguments passed into them, each argument being for a different module's event handler, like this:

//main.js
require("./eventLoader.js")(chatbot, twitter, etc);
//and each of those has it's own set of events that get loaded

//eventLoader.js
module.exports = (chatbot, twitter, etc) => {
  chatbot.on("message", (msg) => reqEvent("message")(chatbot, msg));
  twitter.on("tweet", (tweet) => reqEvent("tweet")(tweet));
  //etc.
};

I think it may just make more sense for users to keep the events all in the same file as when the game instance is declared, because of the way the events are attached to each game instance. That isn't really a big issue because there isn't too much that you would need to do in each event to continue the game, so it shouldn't be so cluttered that you need them in separate files, but I wanted to point out the event loader and separate files because if someone wanted to do that, the current lib doesn't make doing so very easy.

Edit: Maybe it would make more sense if the players weren't passed into the game so early. They could be passed into the game instance when starting a new game, instead of when creating the game instance.

const { Game } = require("uno-engine");
const game = Game();
game.newGame(players);

Then the game instance never has to be overwritten, and you can just start new games with different sets of players instead of redeclaring the game instance every time.

@danguilherme danguilherme added the house-rule Related to custom rules that are not in the game's core label Jun 13, 2018
@danguilherme
Copy link
Owner

danguilherme commented Jun 13, 2018

I implemented a draft of how house rules would be implemented and used in the engine on PR #12.

The first step was to create a kind of cancelable EventEmitter, so house rules implementors may ask the engine to stop its common flow. If the event handler returns false, then the default behaviour must not occur, like in jQuery's .on().

Then we can setup our plugin to listen to specific events and take over whenever they need.

The code needs improvements, so I won't merge the PR right away, but the general idea is there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
house-rule Related to custom rules that are not in the game's core
Projects
None yet
Development

No branches or pull requests

2 participants