Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion deploy/database/schema.game.sql
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ CREATE TABLE game (
last_winner_id SMALLINT UNSIGNED,
tournament_id SMALLINT UNSIGNED,
tournament_round_number SMALLINT UNSIGNED,
description VARCHAR(255) NOT NULL,
description VARCHAR(305) NOT NULL,
chat TEXT,
previous_game_id MEDIUMINT UNSIGNED,
FOREIGN KEY (previous_game_id) REFERENCES game(id)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE game MODIFY description VARCHAR(305) NOT NULL;
7 changes: 7 additions & 0 deletions src/api/DummyApiResponder.php
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,13 @@ protected function get_interface_response_loadGameData($args) {
);
}

protected function get_interface_response_loadTournamentData($args) {
return $this->load_json_data_from_file(
'loadTournamentData',
$args['tournament'] . '.json'
);
}

protected function get_interface_response_countPendingGames() {
return $this->load_json_data_from_file(
'countPendingGames',
Expand Down
12 changes: 8 additions & 4 deletions src/engine/BMInterfaceTournament.php
Original file line number Diff line number Diff line change
Expand Up @@ -503,17 +503,21 @@ protected function generate_new_games(BMTournament $tournament) {
array($gameData['buttonId1'], $gameData['buttonId2'])
);

$description = 'Round ' . $gameData['roundNumber'];
if ('' != $tournament->description) {
$description = $tournament->description . ' ' . $description;
$roundDescription = 'Tournament Round ' . $gameData['roundNumber'];
$tournDescription = $tournament->description;

if ('' == trim($tournDescription)) {
$tournDescription = $roundDescription;
} else {
$tournDescription = $tournDescription . ' • ' . $roundDescription;
}

$interfaceResponse = $this->game()->create_game_from_button_ids(
array($gameData['playerId1'], $gameData['playerId2']),
array($gameData['buttonId1'], $gameData['buttonId2']),
$buttonNames,
$tournament->gameMaxWins,
$description,
$tournDescription,
NULL,
0, // needs to be non-null, but also a non-player ID
TRUE,
Expand Down
124 changes: 124 additions & 0 deletions src/ui/js/Env.js
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,130 @@ Env.applyBbCodeToHtml = function(htmlToParse) {
return outputHtml;
};

Env.removeBbCodeFromHtml = function(htmlToParse) {
// This is all rather more complicated than one might expect, but any attempt
// to parse BB code using simple regular expressions rather than tokenization
// is in the same family as parsing HTML with regular expressions, which
// summons Zalgo.
// (See: http://stackoverflow.com/
// questions/1732348/regex-match-open-tags-except-xhtml-self-contained-tags)

var replacements = {
'b': {},
'i': {},
'u': {},
's': {},
'code': {},
'spoiler': {},
'quote': {},
'game': {
'isAtomic': true,
},
'player': {
'isAtomic': true,
},
'button': {
'isAtomic': true,
},
'set': {
'isAtomic': true,
},
'tourn': {
'isAtomic': true,
},
'wiki': {
'isAtomic': true,
},
'issue': {
'isAtomic': true,
},
'forum': {},
'[': {
'isAtomic': true,
},
};

var outputHtml = '';
var tagStack = [];

// We want to build a pattern that we can use to identify any single
// BB code start tag
var allStartTagsPattern = '';
$.each(replacements, function(tagName) {
if (allStartTagsPattern !== '') {
allStartTagsPattern += '|';
}
// Matches, e.g., '[ b ]' or '[game = "123"]'
// The (?:... part means that we want parentheses around the whole
// thing (so we we can OR it together with other ones), but we don't
// want to capture the value of the whole thing as a group
allStartTagsPattern +=
'(?:\\[(' + Env.escapeRegexp(tagName) + ')(?:=([^\\]]*?))?])';
});

var tagName;

while (htmlToParse) {
var currentPattern = allStartTagsPattern;
if (tagStack.length !== 0) {
// The tag that was most recently opened
tagName = tagStack[tagStack.length - 1];
// Matches '[/i]' et al.
// (so that we can spot the end of the current tag as well)
currentPattern +=
'|(?:\\[(/' + Env.escapeRegexp(tagName) + ')])';
}
// The first group should be non-greedy (hence the ?), and the last one
// should be greedy, so that nested tags work right
// (E.g., in '...blah[/quote] blah [/quote] blah', we want the first .*
// to end at the first [/quote], not the second)
currentPattern = '^(.*?)(?:' + currentPattern + ')(.*)$';
// case-insensitive, multi-line
var regExp = new RegExp(currentPattern, 'im');

var match = htmlToParse.match(regExp);
if (match) {
var stuffBeforeTag = match[1];
// javascript apparently believes that capture groups that don't
// match anything are just important as those that do. So we need
// to do some acrobatics to find the ones we actually care about.
// (match[0] is the whole matched string; match[1] is the stuff before
// the tag. So we start with match[2].)
tagName = '';
for (var i = 2; i < match.length; i++) {
tagName = match[i];
if (tagName) {
break;
}
}
tagName = tagName.toLowerCase();
var stuffAfterTag = match[match.length - 1];

outputHtml += stuffBeforeTag;
if (tagName.substring(0, 1) === '/') {
// If we've found our closing tag, we can finish the current tag and
// pop it off the stack
tagName = tagStack.pop();
} else {
if (!replacements[tagName].isAtomic) {
// If there's a closing tag coming along later, push this tag
// on the stack so we'll know we're waiting on it
tagStack.push(tagName);
}
}

htmlToParse = stuffAfterTag;
} else {
// If we don't find any more BB code tags that we're interested in,
// then we must have reached the end
outputHtml += htmlToParse;
htmlToParse = '';
}
}

return outputHtml;
};

Env.escapeRegexp = function(str) {
return str.replace(/([.?*+^$[\]\\(){}|-])/g, '\\$1');
};
Expand Down
2 changes: 1 addition & 1 deletion src/ui/js/Game.js
Original file line number Diff line number Diff line change
Expand Up @@ -1836,7 +1836,7 @@ Game.pageAddGameHeader = function(action_desc) {

if (Api.game.description) {
Game.page.append($('<div>', {
'text': Api.game.description,
'html': Env.applyBbCodeToHtml(Api.game.description),
'class': 'gameDescDisplay',
}));
}
Expand Down
8 changes: 5 additions & 3 deletions src/ui/js/Overview.js
Original file line number Diff line number Diff line change
Expand Up @@ -443,9 +443,11 @@ Overview.addScoreCol = function(gameRow, gameInfo) {

Overview.addDescCol = function(gameRow, description) {
var descText = '';
if (typeof(description) == 'string') {
descText = description.substring(0, 30) +
((description.length > 30) ? '...' : '');
if (typeof(description) === 'string') {
var descriptionNoMarkup = Env.removeBbCodeFromHtml(description);
Copy link
Contributor

Choose a reason for hiding this comment

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

My understanding is that this diff is changing the behavior of the GAME table on the Overview page.

Is my understanding correct, or does this diff have something to do with tournament behavior?

Copy link
Contributor Author

@blackshadowshade blackshadowshade Jan 4, 2026

Choose a reason for hiding this comment

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

Your understanding is correct, we would be stripping all BBCode from descriptions of games on the Overview page.

Copy link
Contributor Author

@blackshadowshade blackshadowshade Jan 4, 2026

Choose a reason for hiding this comment

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

The stripping is to avoid the problem where the Overview shows something like this:

[forum=1331,32731]Description ...

instead of

Description with extra words

Copy link
Contributor

@cgolubi1 cgolubi1 Jan 4, 2026

Choose a reason for hiding this comment

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

Your understanding is correct, we would be stripping all BBCode from descriptions of games on the Overview page.

Okay. I have two points here, which are basically about documenting the behavior changes in PRs to make them easier to review:

  1. I know this PR is WIP, but the description doesn't say word one about changing the behavior of the games table. The description of a PR should describe the player-visible changes that appear in the PR. The reason it takes me so long to review these PRs is that i (i think correctly) assume there are a ton of player-visible changes which are not called out in the description, so i have to read the code and reverse-engineer them all before having an opinion about whether they are good. The more the PR description says what the intended player-visible changes are, the more we can skip to the second part.
  2. The games table has worked this way for a long time. It's not a new problem that's being introduced by allowing BBCode in tournament descriptions. That's why i'm questioning the urgency of fixing what shows up in these overview tables --- because afaik it's not a new problem. If it's something people have complained about, that's great evidence to put in the description of the issue being resolved.

Copy link
Contributor Author

@blackshadowshade blackshadowshade Jan 4, 2026

Choose a reason for hiding this comment

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

Re 1.: I had thought that by doing things the way I am, I was attempting to preserve the status quo, namely that the Overview page shows the description of the game, instead of showing a whole pile of BBCode that is obscuring the description of the game. I had not considered the fact that the change would be player-visible in the case when there was BBCode in the game title because I have never seen a game description with BBCode in any of my games.

Re 2.: I would argue that this pull request without this adaptation to the Overview page is actually creating a new player-visible problem because I have never seen a game description with BBCode before this pull request, but I am likely to see one after this pull request.


descText = descriptionNoMarkup.substring(0, 30) +
((descriptionNoMarkup.length > 30) ? '...' : '');
}
gameRow.append($('<td>', {
'class': 'gameDescDisplay',
Expand Down
38 changes: 28 additions & 10 deletions src/ui/js/Tournament.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,13 +166,7 @@ Tournament.pageAddTournamentHeader = function() {
// bgcolor = Tournament.color.opponent;
// }

if (Api.tournament.description) {
Tournament.page.append($('<div>', {
'text': Api.tournament.description,
'class': 'gameDescDisplay',
}));
}

Tournament.pageAddTournamentDescription();
Tournament.page.append($('<br>'));

Tournament.pageAddTournamentInfo();
Expand Down Expand Up @@ -306,8 +300,23 @@ Tournament.formFollowTournament = function(e) {
Tournament.showLoggedInPage);
};

Tournament.pageAddTournamentDescription = function () {
if (Api.tournament.description) {
Tournament.page.append($('<div>', {
'id': 'tournament_desc',
'html': Env.applyBbCodeToHtml(Api.tournament.description),
'class': 'gameDescDisplay',
}));
}
};

Tournament.pageAddTournamentInfo = function () {
var infoDiv = $('<div>');
var infoDiv = $(
'<div>',
{
'id': 'tournament_info',
}
);
Tournament.page.append(infoDiv);

var tournamentTypePar = $('<p>', {
Expand Down Expand Up @@ -347,9 +356,18 @@ Tournament.pageAddWinnerInfo = function () {
var winnerDiv = $('<div>');
Tournament.page.append(winnerDiv);

var winnerIdx = Api.tournament.remainCountArray.findIndex(
function(x) {return (x > 0);}
var winnerIdx;
var isWinnerFound = Api.tournament.remainCountArray.some(
function(remainCount, idx) {
winnerIdx = idx;
return remainCount > 0;
}
);

if (!isWinnerFound) {
return;
}

var winnerPar = $('<p>', {
'class': 'winner_name',
'text': 'Winner: ' + Api.tournament.playerDataArray[winnerIdx].playerName
Expand Down
6 changes: 4 additions & 2 deletions src/ui/js/TournamentOverview.js
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,10 @@ TournamentOverview.addTypeCol = function(tournamentRow, tournamentInfo) {
TournamentOverview.addDescCol = function(tournamentRow, description) {
var descText = '';
if (typeof(description) === 'string') {
descText = description.substring(0, 30) +
((description.length > 30) ? '...' : '');
var descriptionNoMarkup = Env.removeBbCodeFromHtml(description);

descText = descriptionNoMarkup.substring(0, 30) +
((descriptionNoMarkup.length > 30) ? '...' : '');
}
tournamentRow.append($('<td>', {
'class': 'tournamentDescDisplay',
Expand Down
6 changes: 3 additions & 3 deletions test/src/api/responderTournamentTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ public function test_interface_tournament() {
$gameOneExpData = $this->generate_init_expected_data_array($gameOneId, 'responder004', 'responder005', 1, 'SPECIFY_DICE');
$gameOneExpData['tournamentId'] = $tournamentId;
$gameOneExpData['tournamentRoundNumber'] = 1;
$gameOneExpData['description'] = 'Round 1';
$gameOneExpData['description'] = 'Tournament Round 1';
$gameOneExpData['currentPlayerIdx'] = FALSE;
$gameOneExpData['creatorDataArray'] = array('creatorId' => 0, 'creatorName' => '');
$gameOneExpData['gameActionLog'][0]['message'] = 'Game created automatically';
Expand Down Expand Up @@ -243,7 +243,7 @@ public function test_interface_tournament() {
$gameTwoExpData['gameSkillsInfo'] = $this->get_skill_info(array('Poison'));
$gameTwoExpData['tournamentId'] = $tournamentId;
$gameTwoExpData['tournamentRoundNumber'] = 1;
$gameTwoExpData['description'] = 'Round 1';
$gameTwoExpData['description'] = 'Tournament Round 1';
$gameTwoExpData['activePlayerIdx'] = 0;
$gameTwoExpData['playerWithInitiativeIdx'] = 0;
$gameTwoExpData['creatorDataArray'] = array('creatorId' => 0, 'creatorName' => '');
Expand Down Expand Up @@ -363,7 +363,7 @@ public function test_interface_tournament() {
$gameThreeExpData['gameSkillsInfo'] = $this->get_skill_info(array('Poison'));
$gameThreeExpData['tournamentId'] = $tournamentId;
$gameThreeExpData['tournamentRoundNumber'] = 2;
$gameThreeExpData['description'] = 'Round 2';
$gameThreeExpData['description'] = 'Tournament Round 2';
$gameThreeExpData['activePlayerIdx'] = 1;
$gameThreeExpData['playerWithInitiativeIdx'] = 1;
$gameThreeExpData['currentPlayerIdx'] = 1;
Expand Down
1 change: 1 addition & 0 deletions test/src/engine/BMInterfaceTournamentTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,5 @@ public function test_create_tournament(
) {

}

}
15 changes: 15 additions & 0 deletions test/src/ui/js/BMTestUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,13 @@ BMTestUtils.getAllElements = function() {
'Loader': JSON.stringify(Loader, null, " "),
'Login': JSON.stringify(Login, null, " "),
'Newgame': JSON.stringify(Newgame, null, " "),
'Newtournament': JSON.stringify(Newtournament, null, " "),
'Newuser': JSON.stringify(Newuser, null, " "),
'OpenGames': JSON.stringify(OpenGames, null, " "),
'Overview': JSON.stringify(Overview, null, " "),
'Profile': JSON.stringify(Profile, null, " "),
'Tournament': JSON.stringify(Tournament, null, " "),
'TournamentOverview': JSON.stringify(TournamentOverview, null, " "),
'UserPrefs': JSON.stringify(UserPrefs, null, " "),
'Verify': JSON.stringify(Verify, null, " "),
};
Expand Down Expand Up @@ -132,6 +135,14 @@ BMTestUtils.testGameId = function(gameDesc) {
if (gameDesc == 'NOGAME') { return '10000000'; }
};

// For each tournament reported by responderTest which we use in UI
// tests, set a friendly name for tracking purposes. These values
// need to be kept in sync with responderTest in order for anything
// good to happen.
BMTestUtils.testTournamentId = function(tournamentDesc) {
if (tournamentDesc == 'default') { return '1'; }
};

// We don't currently usually test reading the URL bar contents, because
// that's hard to do within QUnit, but rather override those contents
// with hardcoded values that we want to test.
Expand All @@ -143,6 +154,10 @@ BMTestUtils.overrideGetParameterByName = function() {
return BMTestUtils.testGameId(BMTestUtils.GameType);
}

if (name == 'tournament') {
return BMTestUtils.testTournamentId(BMTestUtils.TournamentType);
}

// always return the userid associated with tester1 in the fake data
if (name == 'id') {
return '1';
Expand Down
8 changes: 7 additions & 1 deletion test/src/ui/js/test_Env.js
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,12 @@ test("test_Env.applyBbCodeToHtml", function(assert) {
assert.ok(holder.find('.chatItalic').length == 1, '[i] tag should be converted to HTML');
});

test("test_Env.removeBbCodeFromHtml", function(assert) {
var rawHtml = '<b>HTML</b><br/>[i]BB Code[/i]';
var newHtml = Env.removeBbCodeFromHtml(rawHtml);
assert.equal(newHtml, '<b>HTML</b><br/>BB Code', 'Stripped-down HTML should be correct');
});

test("test_Env.escapeRegexp", function(assert) {
var rawText = 'example.com';
var escapedPattern = Env.escapeRegexp(rawText);
Expand Down Expand Up @@ -339,7 +345,7 @@ test("test_Env.toggleSpoiler", function(assert) {
var spoiler = $('<span>', { 'class': 'chatSpoiler' });
var eventTriggerSpan = {'target': {'tagName': 'span'}};
var eventTriggerAnchor = {'target': {'tagName': 'a'}};

Env.toggleSpoiler.call(spoiler, eventTriggerSpan);
assert.ok(spoiler.hasClass('chatExposedSpoiler'),
'Spoiler should be styled as revealed');
Expand Down
2 changes: 1 addition & 1 deletion test/src/ui/js/test_Newtournament.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ module("Newtournament", {
delete Api.player;
delete Newtournament.page;
delete Newtournament.form;
// delete Newtournament.justCreatedGame;
delete Newtournament.justCreatedTournament;

Login.pageModule = null;
Newtournament.activity = {};
Expand Down
Loading