Skip to content

Commit

Permalink
Fix Matchmaker hotpatching
Browse files Browse the repository at this point in the history 
Matchmaker is now globally accessible under Ladders, rather than
needing to be manually required.

This fixes a crash when hotpatching certain things.
  • Loading branch information
@Zarel
Zarel committed Sep 1, 2017
1 parent 7ba80db commit 7a9310a
Show file tree
Hide file tree
Showing 6 changed files with 9 additions and 21 deletions.
10 changes: 4 additions & 6 deletions chat-commands.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@
const crypto = require('crypto');
const FS = require('./fs');

const Matchmaker = require('./ladders-matchmaker').matchmaker;

const MAX_REASON_LENGTH = 300;
const MUTE_LENGTH = 7 * 60 * 1000;
const HOURMUTE_LENGTH = 60 * 60 * 1000;
Expand Down Expand Up @@ -2112,7 +2110,7 @@ exports.commands = {

let entry = targetUser.name + " was forced to choose a new name by " + user.name + (reason ? ": " + reason : "");
this.privateModCommand("(" + entry + ")");
Matchmaker.cancelSearch(targetUser);
Ladders.matchmaker.cancelSearch(targetUser);
targetUser.resetName(true);
targetUser.send("|nametaken||" + user.name + " considers your name inappropriate" + (reason ? ": " + reason : "."));
return true;
Expand Down Expand Up @@ -2142,7 +2140,7 @@ exports.commands = {
}

this.globalModlog("NAMELOCK", targetUser, ` by ${user.name}${reasonText}`);
Matchmaker.cancelSearch(targetUser);
Ladders.matchmaker.cancelSearch(targetUser);
Punishments.namelock(targetUser, null, null, reason);
targetUser.popup(`|modal|${user.name} has locked your name and you can't change names anymore${reasonText}`);
return true;
Expand Down Expand Up @@ -3301,9 +3299,9 @@ exports.commands = {
return false;
}
}
Matchmaker.searchBattle(user, target);
Ladders.matchmaker.searchBattle(user, target);
} else {
Matchmaker.cancelSearch(user, target);
Ladders.matchmaker.cancelSearch(user, target);
}
},

Expand Down
1 change: 1 addition & 0 deletions ladders-remote.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
'use strict';

let Ladders = module.exports = getLadder;
Object.assign(Ladders, require('./ladders-matchmaker'));

Ladders.get = Ladders;
Ladders.formatsListPrefix = '';
Expand Down
1 change: 1 addition & 0 deletions ladders.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
const FS = require('./fs');

let Ladders = module.exports = getLadder;
Object.assign(Ladders, require('./ladders-matchmaker'));

Ladders.get = Ladders;

Expand Down
5 changes: 2 additions & 3 deletions rooms.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ const REPORT_USER_STATS_INTERVAL = 10 * 60 * 1000;
const CRASH_REPORT_THROTTLE = 60 * 60 * 1000;

const FS = require('./fs');
const Matchmaker = require('./ladders-matchmaker').matchmaker;

let Rooms = module.exports = getRoom;

Expand Down Expand Up @@ -1455,8 +1454,8 @@ Rooms.createBattle = function (format, options) {
if (p1 === p2) throw new Error(`Players can't battle themselves`);
if (!p1) throw new Error(`p1 required`);
if (!p2) throw new Error(`p2 required`);
Matchmaker.cancelSearch(p1);
Matchmaker.cancelSearch(p2);
Ladders.matchmaker.cancelSearch(p1);
Ladders.matchmaker.cancelSearch(p2);

if (Rooms.global.lockdown === true) {
p1.popup("The server is restarting. Battles will be available again in a few minutes.");
Expand Down
9 changes: 0 additions & 9 deletions test/application/rooms.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

const assert = require('assert');

const {matchmaker, Matchmaker} = require('../../ladders-matchmaker');
const {User} = require('../../dev-tools/users-utils');

describe('Rooms features', function () {
Expand All @@ -27,11 +26,6 @@ describe('Rooms features', function () {
const packedTeam = 'Weavile||lifeorb||swordsdance,knockoff,iceshard,iciclecrash|Jolly|,252,,,4,252|||||';

let room;
before(function () {
Rooms.global.ladderIpLog.end();
clearInterval(matchmaker.periodicMatchInterval);
matchmaker.periodicMatchInterval = null;
});
afterEach(function () {
Users.users.forEach(user => {
room.onLeave(user);
Expand All @@ -40,9 +34,6 @@ describe('Rooms features', function () {
});
if (room) room.destroy();
});
after(function () {
Object.assign(matchmaker, new Matchmaker());
});

it('should allow two users to join the battle', function () {
let p1 = new User();
Expand Down
4 changes: 1 addition & 3 deletions users.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ const PERMALOCK_CACHE_TIME = 30 * 24 * 60 * 60 * 1000;

const FS = require('./fs');

const Matchmaker = require('./ladders-matchmaker').matchmaker;

let Users = module.exports = getUser;

/*********************************************************
Expand Down Expand Up @@ -1343,7 +1341,7 @@ class User {
}));
}
cancelSearch(format) {
return Matchmaker.cancelSearch(this, format);
return Ladders.matchmaker.cancelSearch(this, format);
}
makeChallenge(user, format, team/*, isPrivate*/) {
user = getUser(user);
Expand Down

6 comments on commit 7a9310a

@Slayer95
Copy link
Contributor

@Slayer95 Slayer95 commented on 7a9310a Sep 2, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

It's likely that this is the result of nodejs/node#14132 actually being a breaking change for hotpatches (first released in Node v8.3.0)

@Slayer95
Copy link
Contributor

Choose a reason for hiding this comment

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

I had a look to both that PR and the PS code to make sure that it doesn't harm us, but I guess my review wasn't careful enough.

@Slayer95
Copy link
Contributor

Choose a reason for hiding this comment

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

The right fix would be to update Chat.uncacheTree so that, while going downwards the tree, only modules with the expected .parent are removed from the cache.

@Zarel
Copy link
Member Author

@Zarel Zarel commented on 7a9310a Sep 2, 2017

Choose a reason for hiding this comment

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

Well, the way Matchmaker was previously implemented was incredibly weird/dirty, anyway. I think this change fixes everything like that?

@Slayer95
Copy link
Contributor

@Slayer95 Slayer95 commented on 7a9310a Sep 2, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

ProcessManager imports from chat plugins are also affected.
Also FS... Everything module-related really.

@Zarel
Copy link
Member Author

@Zarel Zarel commented on 7a9310a Sep 3, 2017

Choose a reason for hiding this comment

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

Yeah, FS was the only thing I was thinking of, but FS is currently entirely self-contained.

That'd change once I set up the write sync stuff, I guess. Maybe FS should also be a global in the main process.

Please sign in to comment.