Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

makes majority of the entrylist fields optional #278

Merged
merged 2 commits into from
Apr 29, 2024

Conversation

klj613
Copy link
Contributor

@klj613 klj613 commented Apr 14, 2024

PR note: happy to approach this in a different way, as far as I could tell this was the only way. Can't seem to add defaults to JSON structs and doing any default in the frontend would only solve this for the frontend and not the API. In the end omitting was likely better as it keeps the entrylist as-is (from simgrid etc) and rely on the ACC defaults itself.

commit msg:

valid entrylist entries can be quite small, e.g.

     {
       "drivers": [
         {
           "playerID": "...."
         }
       ],
       "isServerAdmin": 1
     }

the above valid entrylist entry is from a simgrid entrylist where all admins are added to the entrylist even though they've not signed up for the race.

currently if these entrylists were imported as-is either via the GUI or API the result would end up as:

    {
      "drivers": [
        {
          "firstName": "",
          "lastName": "",
          "shortName": "",
          "driverCategory": 0,
          "playerID": "...",
          "nationality": 0
        }
      ],
      "raceNumber": 0,
      "forcedCarModel": 0,
      "overrideDriverInfo": 0,
      "isServerAdmin": 1,
      "customCar": "",
      "overrideCarModelForCustomCar": 0,
      "ballastKg": 0,
      "restrictor": 0,
      "defaultGridPosition": 0
    }

The main obvious problem here is '0' for the ForcedCarModel is a porsche car, whilst in reality the entrylist from simgrid isn't forcing a car model, similar issues exist for some of the other settings which are not defaulting to -1 and instead defaulting to 0.

The ideal scenario is for the entrylist to be exactly the same once imported via accweb, however due to the checkboxes this is not really possible (checkboxes are either unchecked or checked via the GUI).

You could import via the API and get:

     {
       "drivers": [
         {
           "playerID": "...."
         }
       ],
       "isServerAdmin": 1
     }

However if you import via the GUI you will get:

    {
      "drivers": [
        {
          "playerID": "..."
        }
      ],
      "overrideDriverInfo": 0,
      "isServerAdmin": 1,
      "overrideCarModelForCustomCar": 0
    }

To be consistent I've instead opted to not set omitempty on OverrideDriverInfo and OverrideCarModelForCustomCar which will keep the API and GUI consistent with each other to make future debugging/understanding easier. I don't foresee any issues with importing a simgrid entrylist and having overrideDriverInfo and overrideCarModelForCustomCar added by accweb unless ACC defaults in the future changes (very unlikely).

klj613 and others added 2 commits April 14, 2024 17:04
valid entrylist entries can be quite small, e.g.

```
     {
       "drivers": [
         {
           "playerID": "...."
         }
       ],
       "isServerAdmin": 1
     }
```

the above valid entrylist entry is from a simgrid entrylist where all admins are added to the entrylist
even though they've not signed up for the race.

currently if these entrylists were imported as-is either via the GUI or API the result would end up as:

```
    {
      "drivers": [
        {
          "firstName": "",
          "lastName": "",
          "shortName": "",
          "driverCategory": 0,
          "playerID": "...",
          "nationality": 0
        }
      ],
      "raceNumber": 0,
      "forcedCarModel": 0,
      "overrideDriverInfo": 0,
      "isServerAdmin": 1,
      "customCar": "",
      "overrideCarModelForCustomCar": 0,
      "ballastKg": 0,
      "restrictor": 0,
      "defaultGridPosition": 0
    }
```

The main obvious problem here is '0' for the ForcedCarModel is a porsche car, whilst in reality the entrylist
from simgrid isn't forcing a car model, similar issues exist for some of the other settings which are not
defaulting to `-1` and instead defaulting to `0`.

The ideal scenario is for the entrylist to be exactly the same once imported via accweb, however due to the
checkboxes this is not really possible (checkboxes are either unchecked or checked via the GUI).

You could import via the API and get:

```
     {
       "drivers": [
         {
           "playerID": "...."
         }
       ],
       "isServerAdmin": 1
     }
```

However if you import via the GUI you will get:

```
    {
      "drivers": [
        {
          "playerID": "..."
        }
      ],
      "overrideDriverInfo": 0,
      "isServerAdmin": 1,
      "overrideCarModelForCustomCar": 0
    }
```

To be consistent I've instead opted to not set `omitempty` on `OverrideDriverInfo` and `OverrideCarModelForCustomCar`
which will keep the API and GUI consistent with each other to make future debugging/understanding easier. I don't
foresee any issues with importing a simgrid entrylist and having `overrideDriverInfo` and `overrideCarModelForCustomCar`
added by accweb unless ACC defaults in the future changes (very unlikely).
@pedrofaria
Copy link
Collaborator

hey @klj613 , I just added some code to save the json properly.... let me know if you agree....

@pedrofaria pedrofaria self-assigned this Apr 26, 2024
@klj613
Copy link
Contributor Author

klj613 commented Apr 28, 2024

Hi @pedrofaria ,

Thanks for looking into this. I agree with the changes you've added to the PR :)

@pedrofaria pedrofaria merged commit 014eb8e into assetto-corsa-web:master Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants