Multiplex the behaviour of the /v save command, remove level restrictions

RussellLVP · RussellLVP · commit bf758a6d0d02 · 2020-07-14T14:34:07.000+01:00
diff --git a/data/messages.json b/data/messages.json
@@ -546,12 +546,13 @@
   "VEHICLE_LOCK_UNREGISTERED": "@error Sorry, only registered players can lock vehicles. Register now on www.sa-mp.nl!",
   "VEHICLE_LOCKED": "@success The %s has been locked!",
   "VEHICLE_NOT_DRIVING": "@error %s is not currently in a vehicle managed by JavaScript.",
-  "VEHICLE_NOT_DRIVING_SELF": "@error You need to be in a public vehicle in order to use this command.",
+  "VEHICLE_NOT_DRIVING_SELF": "@error You need to be in a vehicle in order to use this command.",
   "VEHICLE_QUICK_ALREADY_DRIVING": "@error Sorry, but you're already driving a vehicle!",
   "VEHICLE_QUICK_COLLECTABLES": "@error Keep tagging Spray Tags and shooting Red Barrels to enable this command!",
   "VEHICLE_RESET": "@success The vehicle layout has been reset.",
   "VEHICLE_RESPAWNED": "@success The %s has been respawned.",
   "VEHICLE_RESPAWN_NOT_IN_VEHICLE": "@error %s is not in a vehicle, which can thus not be respawned.",
+  "VEHICLE_SAVE_HELP": "@error You can only save vehicles when you're an administrator, have a house, or have VIP rights.",
   "VEHICLE_SAVE_TOO_BUSY": "@error Sorry, this area is too busy! There already are %d vehicles (max: %d) and %d models (max: %d).",
   "VEHICLE_SAVED": "@success The %s has been %s in the database.",
   "VEHICLE_SEIZE_DRIVER": "@error %s is already driving your vehicle!",
diff --git a/javascript/features/vehicles/README.md b/javascript/features/vehicles/README.md
@@ -28,6 +28,7 @@ Furthermore, administrators are able to use the following commands:
   * **/v density**: Displays the density of vehicles within streaming radius around you.
   * **/v enter [seat?]**: Enters the vehicle closest to you, optionally in `seat` (0-8).
   * **/v reset**: Resets the server to its original vehicle layout.
+  * **/v save**: Saves the vehicle that you're currently driving in the database.
   * **/v [player]? access [players/vips/administrators/management]?**: Restricts the vehicle to a
     particular `level`.
   * **/v [player]? color [0-255]? [0-255]?**: Displays or updates the colors of either your own vehicle, or
@@ -36,7 +37,6 @@ Furthermore, administrators are able to use the following commands:
   * **/v [player]? health [0-1000]?**: Displays or updates the health of either your own vehicle, or
     that of `player`.
   * **/v [player]? respawn**: Respawns either your own vehicle, or that of `player`.
-  * **/v [player]? save**: Saves either your own vehicle, or that of `player`, in the database.
 
 _Note that temporary administrators are not allowed to use either `/v save` or `/v delete`._
 
diff --git a/javascript/features/vehicles/vehicle_command_delegate.js b/javascript/features/vehicles/vehicle_command_delegate.js
@@ -0,0 +1,12 @@
+// Copyright 2020 Las Venturas Playground. All rights reserved.
+// Use of this source code is governed by the MIT license, a copy of which can
+// be found in the LICENSE file.
+
+// Delegate for vehicle commands. Not all vehicles manageable are created by the Vehicles system,
+// some are created by other systems such as Houses or gang zones. They can still benefit from the
+// ability to save and change vehicles.
+export class VehicleCommandDelegate {
+    // Called when the |player| wishes to save the vehicle they're driving. Must return a sequence
+    // of options ({ label, listener }) that can be considered by the system.
+    async getVehicleSaveCommandOptions(player) { return []; }
+}
diff --git a/javascript/features/vehicles/vehicle_commands.js b/javascript/features/vehicles/vehicle_commands.js
@@ -37,6 +37,7 @@ const kQuickVehicleCommands = {
 class VehicleCommands {
     constructor(manager, announce, collectables, limits, playground, streamer) {
         this.manager_ = manager;
+        this.delegates_ = new Set();
 
         this.announce_ = announce;
         this.collectables_ = collectables;
@@ -60,10 +61,9 @@ class VehicleCommands {
         }
 
         // Command: /v [vehicle]?
-        //          /v [enter/help/reset]
-        //          /v [player]? [delete/health/respawn/save]
+        //          /v [enter/help/reset/save]
+        //          /v [player]? [delete/health/respawn]
         server.commandManager.buildCommand('v')
-            .restrict(player => this.playground_().canAccessCommand(player, 'v'))
             .sub('enter')
                 .restrict(Player.LEVEL_ADMINISTRATOR)
                 .parameters([ { name: 'seat', type: CommandBuilder.NUMBER_PARAMETER,
@@ -74,6 +74,8 @@ class VehicleCommands {
             .sub('reset')
                 .restrict(Player.LEVEL_MANAGEMENT)
                 .build(VehicleCommands.prototype.onVehicleResetCommand.bind(this))
+            .sub('save')
+                .build(VehicleCommands.prototype.onVehicleSaveCommand.bind(this))
             .sub(CommandBuilder.PLAYER_PARAMETER, player => player)
                 .sub('delete')
                     .restrict(Player.LEVEL_ADMINISTRATOR, /* restrictTemporary= */ true)
@@ -86,15 +88,17 @@ class VehicleCommands {
                 .sub('respawn')
                     .restrict(Player.LEVEL_ADMINISTRATOR)
                     .build(VehicleCommands.prototype.onVehicleRespawnCommand.bind(this))
-                .sub('save')
-                    .restrict(Player.LEVEL_ADMINISTRATOR, /* restrictTemporary= */ true)
-                    .build(VehicleCommands.prototype.onVehicleSaveCommand.bind(this))
                 .build(/* deliberate fall-through */)
             .sub(CommandBuilder.WORD_PARAMETER)
+                .restrict(player => this.playground_().canAccessCommand(player, 'v'))
                 .build(VehicleCommands.prototype.onVehicleCommand.bind(this))
             .build(VehicleCommands.prototype.onVehicleCommand.bind(this));
     }
 
+    // Either adds or removes the given |delegate| from the set of vehicle command delegates.
+    addCommandDelegate(delegate) { this.delegates_.add(delegate); }
+    removeCommandDelegate(delegate) { this.delegates_.delete(delegate); }
+
     // ---------------------------------------------------------------------------------------------
 
     // Called when the player wants to seize the vehicle that they're in. This will move them to
@@ -350,19 +354,25 @@ class VehicleCommands {
     // Called when the |player| executes `/v help`. Displays more information about the command, as
     // well as the available sub-commands to the |player|.
     onVehicleHelpCommand(player) {
-        player.sendMessage(Message.VEHICLE_HELP_SPAWN);
+        const globalOptions = [ 'save' ];
+        const vehicleOptions = [];
 
-        if (!player.isAdministrator())
-            return;
+        if (player.isAdministrator()) {
+            globalOptions.push('enter', 'help', 'reset');
+            vehicleOptions.push('health', 'respawn');
 
-        const globalOptions = ['enter', 'help', 'reset'];
-        const vehicleOptions = ['health', 'respawn'];
+            if (!player.isTemporaryAdministrator())
+                vehicleOptions.push('delete');
+        }
 
-        if (!player.isTemporaryAdministrator())
-            vehicleOptions.push('delete', 'save');
+        if (this.playground_().canAccessCommand(player, 'v'))
+            player.sendMessage(Message.VEHICLE_HELP_SPAWN);
 
-        player.sendMessage(Message.VEHICLE_HELP_GLOBAL, globalOptions.sort().join('/'));
-        player.sendMessage(Message.VEHICLE_HELP_VEHICLE, vehicleOptions.sort().join('/'));
+        if (globalOptions.length)
+            player.sendMessage(Message.VEHICLE_HELP_GLOBAL, globalOptions.sort().join('/'));
+
+        if (vehicleOptions.length)
+            player.sendMessage(Message.VEHICLE_HELP_VEHICLE, vehicleOptions.sort().join('/'));
     }
 
     // Called when the |player| requests the vehicle layout to be reset.
@@ -395,14 +405,62 @@ class VehicleCommands {
         player.sendMessage(Message.VEHICLE_RESPAWNED, vehicle.model.name);
     }
 
-    // Called when the |player| executes `/v save` or `/v [player] save`, which means they wish to
-    // save the vehicle in the database to make it a persistent vehicle.
-    async onVehicleSaveCommand(player, subject) {
-        const vehicle = subject.vehicle;
+    // Called when the |player| executes `/v save`, which means they wish to save the vehicle in the
+    // database to make it a persistent vehicle. Delegates will also be considered before making
+    // this decision, as players might want to save e.g. their house vehicle.
+    async onVehicleSaveCommand(player) {
+        const vehicle = player.vehicle;
 
-        // Bail out if the |subject| is not driving a vehicle, or it's not managed by this system.
+        // Bail out if the |player| is not currently in a vehicle - there's nothing to save. They
+        // can see general usage guidelines after having entered one.
+        if (!vehicle) {
+            player.sendMessage(Message.VEHICLE_NOT_DRIVING_SELF);
+            return;
+        }
+
+        const options = [];
+
+        // Check whether one of the registered delegates is able to handle the vehicle's save. They
+        // return a sequence of options, which could be displayed in a dialog.
+        for (const delegate of this.delegates_)
+            options.push(...await delegate.getVehicleSaveCommandOptions(player));
+
+        // If the |player| is an administrator and has the ability to save vehicles, add that to the
+        // given |options| as well.
+        if (player.isAdministrator() && !player.isTemporaryAdministrator()) {
+            options.push({
+                label: `Save to the vehicle layout`,
+                listener: VehicleCommands.prototype.onVehiclePermanentlySaveCommand.bind(this),
+            });
+        }
+
+        // There are three options here: (1) no options, show a help message, (2) one option, fast
+        // path and immediately call the listener, or (3) multiple options, show a dialog.
+        if (options.length === 1) return await options[0].listener(player, vehicle);
+
+        if (!options.length) {
+            player.sendMessage(Message.VEHICLE_SAVE_HELP);
+            return;
+        }
+
+        // (1) Sort the |options| based in ascending order based on the label text.
+        options.sort((lhs, rhs) => lhs.label.localeCompare(rhs.label));
+
+        // (2) Compile a dialog with each of the |options|, and have the user pick one instead.
+        const dialog = new Menu('Vehicle options');
+
+        for (const { label, listener } of options)
+            dialog.addItem(label, listener.bind(null, player, vehicle));
+
+        return await dialog.displayForPlayer(player);
+    }
+
+    // Called when the |player|'s |vehicle| has to be permanently saved to the database. This option
+    // is only available to permanent administrators, and will persist between sessions.
+    async onVehiclePermanentlySaveCommand(player, vehicle) {
+        // Bail out if the |player| is not driving a vehicle, or it's not managed by this system.
         if (!this.manager_.isManagedVehicle(vehicle)) {
-            player.sendMessage(Message.VEHICLE_NOT_DRIVING, subject.name);
+            player.sendMessage(Message.VEHICLE_NOT_DRIVING, player.name);
             return;
         }
 
diff --git a/javascript/features/vehicles/vehicle_commands.test.js b/javascript/features/vehicles/vehicle_commands.test.js
@@ -198,18 +198,6 @@ describe('VehicleCommands', (it, beforeEach) => {
         }
     });
 
-    // TODO: We'll actually want to make this available to all the players.
-    // See the following issue: https://github.com/LVPlayground/playground/issues/330
-    it('should limit /v to administrators only', async(assert) => {
-        const russell = server.playerManager.getById(1 /* Russell */);
-        assert.equal(russell.level, Player.LEVEL_PLAYER);
-
-        assert.isTrue(await russell.issueCommand('/v'));
-        assert.equal(russell.messages.length, 1);
-        assert.equal(russell.messages[0],
-                     Message.format(Message.COMMAND_ERROR_INSUFFICIENT_RIGHTS, 'specific people'));
-    });
-
     it('should support spawning vehicles by their model Id', async(assert) => {
         for (const invalidModel of ['-15', '42', '399', '612', '1337']) {
             assert.isTrue(await gunther.issueCommand('/v ' + invalidModel));
@@ -469,7 +457,7 @@ describe('VehicleCommands', (it, beforeEach) => {
 
         assert.isTrue(await gunther.issueCommand('/v save'));
         assert.equal(gunther.messages.length, 1);
-        assert.equal(gunther.messages[0], Message.VEHICLE_QUICK_ALREADY_DRIVING);
+        assert.equal(gunther.messages[0], Message.VEHICLE_SAVE_HELP);
 
         assert.isTrue(await gunther.issueCommand('/v delete'));
         assert.equal(gunther.messages.length, 2);
@@ -480,8 +468,9 @@ describe('VehicleCommands', (it, beforeEach) => {
         gunther.level = Player.LEVEL_PLAYER;
         {
             assert.isTrue(await gunther.issueCommand('/v help'));
-            assert.equal(gunther.messages.length, 1);
+            assert.equal(gunther.messages.length, 2);
             assert.equal(gunther.messages[0], Message.VEHICLE_HELP_SPAWN);
+            assert.equal(gunther.messages[1], Message.format(Message.VEHICLE_HELP_GLOBAL, 'save'));
 
             gunther.clearMessages();
         }
diff --git a/javascript/features/vehicles/vehicles.js b/javascript/features/vehicles/vehicles.js
@@ -49,6 +49,18 @@ class Vehicles extends Feature {
         this.decorations_ = new VehicleDecorations(settings, announce);
     }
 
+    // ---------------------------------------------------------------------------------------------
+    // Public API of the Vehicles feature
+    // ---------------------------------------------------------------------------------------------
+
+    // Adds the given |delegate| to the set of delegates that can handle vehicle commands.
+    addCommandDelegate(delegate) { this.commands_.addCommandDelegate(delegate); }
+
+    // Removes the given |delegate| from the set of delegates that can handle vehicle commands.
+    removeCommandDelegate(delegate) { this.commands_.removeCommandDelegate(delegate); }
+
+    // ---------------------------------------------------------------------------------------------
+
     dispose() {
         this.commands_.dispose();
         this.commands_ = null;
