the-hideout / TarkovMonitor

Monitor Tarkov log files to help track progress, queues, and groups
GNU General Public License v3.0
94 stars 17 forks source link

Ensure profileId is set when testing API token #77

Closed Owaan closed 5 months ago

Owaan commented 5 months ago

Resolves #73

Not sure if the profile ID needs to be mapped to the API key as TarkovTracker isn't separating profiles (yet). This should do the job though 👍

This also fixes the scenario where a user could be confused launching TarkovMonitor before launching Tarkov or having a profile reported in their logs. You'll still need to handle launching TarkovMonitor without even having Tarkov installed though....

Razzmatazzz commented 5 months ago

@Owaan Thank you for this PR! Can you explain a bit more the reasoning behind the changes? Is the current code for associating profiles with tarkov tracker tokens not working as intended?

Owaan commented 5 months ago

@Razzmatazzz All good!

I've fixed two issues in this PR caused by the profileId changes. My general thoughts are that both of these issues are caused by using a dictionary to map a profileId to the API tokens when you might not need to. I have left that the same in my changes incase you had future plans regarding that.

Issue 1:

Fixed by this commit

Testing Steps:

  1. Remove AppData/Local/TarkovMonitor/<path>/<version>/user.config
  2. Launch TarkovMonitor
  3. Paste TarkovTracker API key into Settings API Key
  4. Click "Test Token"

Expected Result: API success response

Actual Result: "TarkovTracker API error: The given key '' was not present in the dictionary."

Solution: The original code here, currentProfile could be an empty string so you'd never find the token in the dictionary.

return Task.Run<string>(() => {
    return tokens[currentProfile];
});

I'm now passing the profileId into the TestToken method.

Issue 2:

Fixed by this commit and this commit

Testing Steps:

  1. Remove AppData/Local/TarkovMonitor/<path>/<version>/user.config
  2. Empty your C:\Battlestate Games\EFT\Logs directory (leaving one log which doesn't include a profileId)
  3. Launch TarkovMonitor

Expected Result: A graceful error indicating the game needs to be started at least once to track progress

Actual Result: An ambiguous error (I can't even recall exactly what it was at this point)

Solution: I've just tried to catch a few places to handle if the profileId couldn't be found.

Razzmatazzz commented 5 months ago

Thanks for the explanation, and for the fixes!

Owaan commented 5 months ago

@Razzmatazzz oh yeah you might be right I think I misread something and also forgot to use the profileId I passed in anyways lol... Will clean it up in a bit, test and recommit

Owaan commented 5 months ago

@Razzmatazzz All changes fixed, first time using C#, apologies that wasn't so clean the first go around

I've just used your existing GetToken method in the AuthorizationHeaderValueGetter method now.

The main issue was in InitializeProgress, we needed to set the TarkovTracker profile before calling TarkovTracker.TestToken()