-
Notifications
You must be signed in to change notification settings - Fork 4
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
Refactor Riichi point calculations #22
Changes from 8 commits
5fd8666
5c07cfb
b4fd46d
25695b8
75df9b0
7f00868
5aff7f1
149c1b1
c2a13da
7f254dd
4ffe150
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,11 +13,23 @@ export const Constants = { | |
// from noten players to tenpai ones in a Japanese game | ||
JPN_TENPAI_PAYOUT: 3000, | ||
|
||
// The required number of points of any one player to | ||
// The required number of points of any one player to | ||
// end a game at the end of "South" round in a Japanese | ||
// game | ||
JPN_END_POINTS: 30000, | ||
|
||
// Base points of a mangan | ||
JPN_MANGAN_BASE_POINTS: 2000, | ||
|
||
// Points for every bonus round | ||
BONUS_POINTS: 300, | ||
|
||
// Points for every riichi stick | ||
RIICHI_POINTS: 1000, | ||
|
||
// Points paid for a mistake | ||
MISTAKE_POINTS: 12000, | ||
|
||
// Placeholder value to establish a player select button | ||
// That has no player selected | ||
NO_PERSON: "no one", | ||
|
@@ -43,7 +55,21 @@ export const Constants = { | |
SELF_DRAW: "selfdraw", | ||
NO_WIN: "nowin", | ||
RESTART: "restart", | ||
MISTAKE: "fuckup" | ||
MISTAKE: "fuckup", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might conflict now |
||
|
||
EAST: "east", | ||
SOUTH: "south", | ||
WEST: "west", | ||
NORTH: "north", | ||
|
||
PRIORITY: { | ||
east: 3, | ||
south: 2, | ||
north: 1, | ||
west: 0 | ||
} | ||
}; | ||
|
||
Constants.WINDS = [Constants.EAST, Constants.SOUTH, Constants.WEST, Constants.NORTH]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this defined outside the Constants object? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Those values don't exist until the Constants object is defined. I added this line afterwards so I didn't have to be redundant. |
||
|
||
Object.keys(Constants).forEach((k) => { Template.registerHelper(k, () => Constants[k] )}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,15 +56,13 @@ export var NewGameUtils = { | |
return "西"; | ||
else //if (round > 12) | ||
return "北"; | ||
break; | ||
case Constants.GAME_TYPE.JAPANESE: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are you so keen on putting breaks in every case? I prefer to not include breaks in switch cases which are returned from. Looks cleaner and the break is never called anyway. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It wasn't me that had them to begin with lmao There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. o im dum |
||
if (round <= 4) | ||
return "東"; | ||
if (round > 4 && round <= 8) | ||
return "南"; | ||
else //if (round > 8) | ||
return "西";; | ||
break; | ||
return "西"; | ||
}; | ||
}, | ||
|
||
|
@@ -98,18 +96,14 @@ export var NewGameUtils = { | |
getFirstPlace() { | ||
let values = []; | ||
// For ties, prioritise players in seating order | ||
let priority = { east: 3, | ||
south: 2, | ||
west: 1, | ||
north: 0 }; | ||
|
||
["east", "south", "west", "north"].forEach(k => { | ||
Constants.WINDS.forEach(k => { | ||
values.push({ wind: k, value: this.getDirectionScore(k) }) | ||
}); | ||
|
||
let winner = values.reduce((a, b) => { | ||
if (a.value == b.value) { | ||
return priority[a["wind"]] > priority[b["wind"]] ? a : b; | ||
return Constants.PRIORITY[a["wind"]] > Constants.PRIORITY[b["wind"]] ? a : b; | ||
} else { | ||
return a.value > b.value ? a : b; | ||
} | ||
|
@@ -129,13 +123,12 @@ export var NewGameUtils = { | |
let westRoundOver = Session.get("current_round") > 12; | ||
// End condition where game has reached the end of south round with at least one player above minimum | ||
let someoneAboveMinimum = Session.get("current_round") > 8 && | ||
this.someoneAboveMinimum(Constants.JPN_END_POINTS); | ||
this.someoneAboveMinimum(Constants.JPN_END_POINTS); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So close, but this doesn't actually line up with the above line. |
||
// End condition where north player reaches first place after winning on last round | ||
let dealerFirstAndAboveMinimum = Session.get("current_round") == 8 && | ||
Session.get("current_bonus") > 0 && | ||
this.getDirectionScore("north") >= Constants.JPN_END_POINTS && | ||
this.getFirstPlace() == "north"; | ||
|
||
Session.get("current_bonus") > 0 && | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above. |
||
this.getDirectionScore("north") >= Constants.JPN_END_POINTS && | ||
this.getFirstPlace() == "north"; | ||
return someoneBankrupt || westRoundOver || someoneAboveMinimum || dealerFirstAndAboveMinimum; | ||
}, | ||
|
||
|
@@ -229,29 +222,29 @@ export var NewGameUtils = { | |
|
||
getDirectionScore(direction) { | ||
switch (direction) { | ||
case "east": | ||
case Constants.EAST: | ||
return Number(Session.get("east_score")); | ||
case "south": | ||
case Constants.SOUTH: | ||
return Number(Session.get("south_score")); | ||
case "west": | ||
case Constants.WEST: | ||
return Number(Session.get("west_score")); | ||
case "north": | ||
case Constants.NORTH: | ||
return Number(Session.get("north_score")); | ||
} | ||
}, | ||
|
||
playerToDirection(player) { | ||
if (player == Session.get("current_east")) return "east"; | ||
if (player == Session.get("current_south")) return "south"; | ||
if (player == Session.get("current_west")) return "west"; | ||
if (player == Session.get("current_north")) return "north"; | ||
if (player == Session.get("current_east")) return Constants.EAST; | ||
if (player == Session.get("current_south")) return Constants.SOUTH; | ||
if (player == Session.get("current_west")) return Constants.WEST; | ||
if (player == Session.get("current_north")) return Constants.NORTH; | ||
}, | ||
|
||
roundToDealerDirection(round) { | ||
if (round % 4 == 1) return "east"; | ||
if (round % 4 == 2) return "south"; | ||
if (round % 4 == 3) return "west"; | ||
if (round % 4 == 0) return "north"; | ||
if (round % 4 == 1) return Constants.EAST; | ||
if (round % 4 == 2) return Constants.SOUTH; | ||
if (round % 4 == 3) return Constants.WEST; | ||
if (round % 4 == 0) return Constants.NORTH; | ||
}, | ||
}; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,132 @@ | ||
import { Constants } from './Constants'; | ||
import { NewGameUtils } from './NewGameUtils'; | ||
|
||
export var PointCalculationUtils = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you're going to do this, make it generic (include HK calculation too it's much more simple anyway) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like to do it later, since this ticket is already quite large. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm just worried about JPN and HKG code diverging too much when it should be converging. |
||
jpn_dealin_delta, | ||
jpn_selfdraw_delta, | ||
jpn_mistake_delta | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, is it possible to keep the jpn object in PointCalculationUtils with members "dealin_delta = jpn_dealin_delta"? It seems you removed it this entry in favour of just the suffix. |
||
}; | ||
|
||
/** | ||
* Calculates the change in points as a result of a deal-in hand | ||
* Uses the formula from https://en.wikipedia.org/wiki/Japanese_Mahjong_scoring_rules | ||
* | ||
* @return {Object} containing the point difference for each seat | ||
*/ | ||
function jpn_dealin_delta(points, fu, winnerWind, loserWind, riichiSticks) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we're adding function headers, make sure to add @param entries |
||
let winds = {}; | ||
winds[Constants.EAST] = winds[Constants.SOUTH] = winds[Constants.WEST] = winds[Constants.NORTH] = 0; | ||
|
||
let basicPoints; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some funky indentation here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll fix that in my next commit. |
||
let currentBonus = Number(Session.get("current_bonus")) * Constants.BONUS_POINTS; | ||
|
||
// The multiplier is for whether or not it's a dealer victory | ||
let multiplier = (winnerWind != NewGameUtils.roundToDealerDirection(Number(Session.get("current_round")))) ? 4 : 6; | ||
|
||
// Check to see if you have to count basic points | ||
if (points < 5) { | ||
if (fu == 20 || (points == 1 && fu == 25)) { | ||
return 0; // Issue Protection | ||
} else { | ||
let manganPayout = Constants.JPN_MANGAN_BASE_POINTS * multiplier; | ||
basicPoints = Math.ceil((fu * Math.pow(2, 2 + points)) * multiplier / 100) * 100; | ||
basicPoints = basicPoints < manganPayout ? basicPoints : manganPayout; | ||
} | ||
} else { basicPoints = manganValue(points) * multiplier }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you're going to do this, the semicolon should be at the end of the line, not the end of the block. I would still make this and the other one like it a seperate line. |
||
|
||
winds[winnerWind] = basicPoints + currentBonus + (riichiSticks * Constants.RIICHI_POINTS); | ||
winds[loserWind] = -(basicPoints + currentBonus); | ||
|
||
Session.set("free_riichi_sticks", 0); | ||
return winds; | ||
}; | ||
|
||
|
||
/** | ||
* Calculates the change in points as a result of a self-drawn hand | ||
* Uses the formula from https://en.wikipedia.org/wiki/Japanese_Mahjong_scoring_rules | ||
* | ||
* @return {Object} containing the point difference for each seat | ||
*/ | ||
function jpn_selfdraw_delta(points, fu, winnerWind, riichiSticks) { | ||
let winds = {}; | ||
let dealerWind = NewGameUtils.roundToDealerDirection(Number(Session.get("current_round"))); | ||
|
||
let basicPoints; | ||
let nonDealerPays; | ||
let dealerPays; | ||
|
||
let currentBonus = Number(Session.get("currentBonus")); | ||
let individualBonusPayout = Constants.BONUS_POINTS / 3; | ||
|
||
winds[Constants.EAST] = winds[Constants.SOUTH] = winds[Constants.WEST] = winds[Constants.NORTH] = 0; | ||
|
||
// Check to see if you have to count basic points | ||
if (points < 5) { | ||
if (points == 1 && (fu == 20 || fu == 25)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You also need to protect 2 point 25 selfdraw as that combination does not exist. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is prevented in NewGameUtils#noIllegalSelfDraw. This should be removed for issue #27. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So will this be removed or augmented with 2p25f? In between makes no sense |
||
return 0; // Issue Protection | ||
} else { | ||
basicPoints = fu * Math.pow(2, 2 + points); | ||
basicPoints = basicPoints < Constants.JPN_MANGAN_BASE_POINTS ? basicPoints : Constants.JPN_MANGAN_BASE_POINTS; | ||
} | ||
} else { basicPoints = manganValue(points); } | ||
|
||
nonDealerPays = Math.ceil(basicPoints / 100 * (dealerWind == winnerWind ? 2 : 1)) * 100; | ||
dealerPays = Math.ceil(basicPoints / 100 * 2) * 100; | ||
|
||
for (let w in winds) { | ||
if (winnerWind != dealerWind) { | ||
if (w == dealerWind) { | ||
winds[w] = -(dealerPays + currentBonus * individualBonusPayout); | ||
} else if (w == winnerWind) { | ||
winds[w] = dealerPays + (nonDealerPays * 2) + (currentBonus * Constants.BONUS_POINTS) + (riichiSticks * Constants.RIICHI_POINTS); | ||
} else { | ||
winds[w] = -(nonDealerPays + currentBonus * individualBonusPayout); | ||
} | ||
} else { | ||
if (w == winnerWind) { | ||
winds[w] = (nonDealerPays * 3) + (currentBonus * Constants.BONUS_POINTS) + (riichiSticks * Constants.RIICHI_POINTS); | ||
} else { | ||
winds[w] = -(nonDealerPays + (currentBonus * individualBonusPayout)); | ||
} | ||
} | ||
} | ||
|
||
Session.set("free_riichi_sticks", 0); | ||
return winds; | ||
}; | ||
|
||
/** | ||
* Calculates the point difference as the result of a mistaken hand | ||
* @return {Object} containing the point difference for each seat | ||
*/ | ||
function jpn_mistake_delta(loser) { | ||
let winds = {}; | ||
winds[Constants.EAST] = winds[Constants.SOUTH] = winds[Constants.WEST] = winds[Constants.NORTH] = Constants.MISTAKE_POINTS / 3; | ||
|
||
winds[loser] = -Constants.MISTAKE_POINTS; | ||
|
||
return winds; | ||
}; | ||
|
||
/** | ||
* Calculates the total base points from han + dora values for high value hands | ||
* @returns {Number} representing number of base points as the result of a certain point threshold | ||
*/ | ||
function manganValue(points) { | ||
switch(points) { | ||
case 5: return Constants.JPN_MANGAN_BASE_POINTS; | ||
case 6: | ||
case 7: return Constants.JPN_MANGAN_BASE_POINTS * 1.5; | ||
case 8: | ||
case 9: | ||
case 10: return Constants.JPN_MANGAN_BASE_POINTS * 2; | ||
case 11: | ||
case 12: return Constants.JPN_MANGAN_BASE_POINTS * 3; | ||
case 13: return Constants.JPN_MANGAN_BASE_POINTS * 4; | ||
case 26: return Constants.JPN_MANGAN_BASE_POINTS * 4 * 2; | ||
case 39: return Constants.JPN_MANGAN_BASE_POINTS * 4 * 3; | ||
case 52: return Constants.JPN_MANGAN_BASE_POINTS * 4 * 4; | ||
case 65: return Constants.JPN_MANGAN_BASE_POINTS * 4 * 5; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please specify this is the Japanese chombo amount. The Hong Kong amount is different.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might as well give the JPN_ prefix to bonus and Riichi too, since that is sort of the convention it seems.