Stargate Rewritten

Stargate Rewritten

241 Downloads

Missing response when player is lacking funds to use a portal

Thorinwasher opened this issue ยท 12 comments

commented

How to replicate:

  • Have economy enabled for stargate
  • set useCost to a value larger than 0
  • set the amount of cash a player has lesser than useCost
  • Enter a portal

Behavior:

  • No teleportation is done, neither does any LACKING_FUNDS message show.

Expected behavior:

  • Player get's the LACKING_FUNDS message
  • Player teleports to the entrance portals exit
  • Player gets turned around 180 degrees
commented

We could just keep a list of all the players that have been charged, then if someone lacked permission, money, or if any other reson we could refund those who got charged. This would avoid the .has method completely. So probably do that in the dfs, and if the dfs returned negative, then refund those who got charged.

Error message should already be dealt with, and we can send that during the teleportation. Note that no money or permission checks will be done during the teleportation in this way.

commented

Okay, I know what's happening, we already have code that deals with this:

TeleportedEntityRelationDFS dfs = new TeleportedEntityRelationDFS((anyEntity) -> {
            //TODO: The access event should be called to allow add-ons cancelling or overriding the teleportation
            StargatePermissionManager permissionManager = new StargatePermissionManager(anyEntity);
            if (!hasPermission(anyEntity, permissionManager)) {
                teleportMessage = permissionManager.getDenyMessage();
                return false;
            }

            if (anyEntity instanceof Player && !Stargate.getEconomyManager().has((Player) anyEntity, this.cost)) {
                teleportMessage = Stargate.getLanguageManagerStatic().getErrorMessage(TranslatableMessage.LACKING_FUNDS);
                return false;
            }
            return true;
        }, nearbyLeashed);

        hasPermission = dfs.depthFirstSearch(baseEntity);
        if (!hasPermission) {
            rotation = Math.PI;
            if (origin != null) {
                exit = origin.getExit().add(new Vector(0.5, 0, 0.5));
            } else {
                exit = baseEntity.getLocation();
            }
        }

So we should just do:

if (target instanceof Player) {
          charge(target)
 }
commented
if (target instanceof Player) {
          charge(target)
 }

The transaction may still fail even if the player has the necessary funds. It will only return if it's not possible to extract the payment from the player.
In this case, you can either abort the entire teleportation at the last minute, or cut off that one player.

commented

Then we probably need to have a boolean field isTeleportationDenied for that to work properly

commented

Then we probably need to have a boolean isTeleportationDenied for that to work properly

Or perhaps take the payment just after running the depth-first search. The only problem is what to do if some players have already paid? We don't know if they paid to the owner, to the tax account or to the air, so we'd need to keep track of this and revert it.

commented

As the transaction fails, even though has() says the player can pay, I fear there might be an underlying bug as well.

We need to handle transaction failed situations, but there seems to be another problem as well, in either VaultEconomyManager.has() or in VaultEconomyManager.chargePlayer().

If you made sure to have less money than the cost, has() should return false, unless Vault isn't loaded.

commented

There is a TODO for this.

commented

We could just keep a list of all the players that have been charged, then if someone lacked permission or money, we could refund those who got charged. This would avoid the .has method completely

These are the different cases we need to keep track of, and which are hidden to us as of now:
If the player paid to the gate owner, run a transaction paying the same sum from the gate owner to the player.
If the player paid to the tax account, run a transaction paying the same sum from the tax account to the player.
If the player paid normally, add the sum to the player's account.
If the payment was skipped, don't do anything.

And there is also the case if the refund fails.

commented

Then we probably need to deal with refunds the same way we withdraw money

commented

Then we probably need to deal with refunds the same way we withdraw money

We should probably separate the process of finding the transaction target and the process of taking payment to prevent copied code. Basically, one method gives the UUID of the payment receiver (null for the void). Then this can be used both to pay to the correct account, and to find the account to take refunds from.

commented

Sure! Sounds like a good idea

commented

Using getTransactionReceiver added in 4bbaf06, you can get the account the refund should be taken from. Remember to only refund from the entities which have successfully paid.