anegostudios / VintageStory-Issues

Vintage Story's public issue tracker for reporting bugs, crashes and the like
43 stars 16 forks source link

Incorrect error output when executing the /land adminfree command #1593

Closed Skif97 closed 1 year ago

Skif97 commented 2 years ago

When executing a command /land freeadmin with a playername whose name does not exist, the authorization server will return the response {"valid":1,"playeruid":null}, but the game will display a message that the authorization server is offline. This is because the 2 functions contradict each other. "FreeLandAdmin" first checks the connection and then the status of the player id. And the function "ResolvePlayerName" first checks the status of the player id, then assigns the status of the connection. But due to the fact that the function "ResolvePlayerName" is executed first, in the function "FreeLandAdmin" the branch

if (playeruid == null){
     this.Error(player, groupId, Lang.Get("Unable to release claims, no such player found", Array.Empty<object>()));
     return;
}

is unreachable.

here is the full code of the functions with highlighted problem areas

public static void ResolvePlayerName(string playername, Action<EnumServerResponse, string> OnResolveComplete)
{
    using (MyWebClient myWebClient = new MyWebClient(10))
    {
        NameValueCollection nameValueCollection = new NameValueCollection();
        nameValueCollection["playername"] = playername;
        Uri uri = new Uri("https://auth.vintagestory.at/resolveplayername");
        myWebClient.PostAsync(uri, nameValueCollection, delegate(CompletedArgs args)
        {
            if (args.State != CompletionState.Good)
            {
                OnResolveComplete(EnumServerResponse.Offline, null);
                return;
            }
            ServerMain.Logger.Debug("Response from auth server: {0}", new object[]
            {
                args.Response
            });
            ResolveResponse resolveResponse = JsonConvert.DeserializeObject<ResolveResponse>(args.Response);
↓↓↓↓            if (resolveResponse.playeruid != null)
            {
                OnResolveComplete(EnumServerResponse.Good, resolveResponse.playeruid);
                return;
        }
            OnResolveComplete(EnumServerResponse.Bad, null);
↑↑↑↑        });
    }
}

private void FreeLandAdmin(IServerPlayer player, int groupId, CmdArgs args)
{
    List<LandClaim> allclaims = this.server.SaveGameData._x9FAa1SSF5aqoBaEw2AIXG3weFq;
    string playername = args.PopWord(null);
    AuthServerComm.ResolvePlayerName(playername, delegate(EnumServerResponse result, string playeruid)
    {
        this.server.EnqueueMainThreadTask(delegate
        {
↓↓↓↓            if (result != EnumServerResponse.Good)
            {
                this.Error(player, groupId, Lang.Get("Cannot free claims, auth server seems offline", Array.Empty<object>()));
                return;
            }
            if (playeruid == null)
            {
                this.Error(player, groupId, Lang.Get("Cannot free claims, no such player found", Array.Empty<object>()));
                return;
↑↑↑↑            }
            int num = 0;
            foreach (LandClaim landClaim in new List<LandClaim>(allclaims))
            {
                if (landClaim.OwnedByPlayerUid == playeruid && this.server.WorldMap.Remove(landClaim))
                {
                    num++;
                }
            }
            this.Success(player, groupId, Lang.Get("Ok, {0} claims from {1} removed", new object[]
            {
                num,
                playername
            }));
        });
    });
}
Skif97 commented 2 years ago

it would also be nice to display a message from the absence of an argument when the /land adminfree [playername] command is called without specifying [playername]

DArkHekRoMaNT commented 1 year ago

No longer relevant in 1.18 (new Command API fix it)