gw2-api / issues

Issue tracker for the Guild Wars 2 API
https://discord.gg/zqeHCEg
16 stars 0 forks source link

Expose floorId and sectorId in MumbleLink context struct #21

Open agaertner opened 2 years ago

agaertner commented 2 years ago

Feature Description

Add floorId and/or sectorId to the mumble link context struct supporting and reducing queries to the very complex v2/continents endpoint.

As discussed in PR #2 of the official API repo floor ids from the API differ per map and are arbitrary! We do not have enough information about the player's coordinates to calculate the current floor. Current floor detection is impossible and thus current sector detection by any means (eg. R-tree) BREAKS on maps with sectors lying ontop of each other.

Possible use-cases for having floorId and/or sectorId in mumble is

  1. popular marker utility tools could utilize it to increase performance by restricting loading markers to the floor or sector greatly increasing accessibility for low-end PCs.
  2. gimmicks such as showing nameplates of entering a sector: https://gfycat.com/CompetentReadyGilamonster
  3. Conviniently trigger a request of sector constrained data from a backend. ("paginating" map constrained data.)

Implementation

The context struct may be adapted like so:

// Mumble Link Context
// Just raw bytes.
struct MumbleContext {
    byte serverAddress[28]; // contains sockaddr_in or sockaddr_in6
    uint32_t mapId;
    uint32_t mapType;
    uint32_t shardId;
    uint32_t instance;
    uint32_t buildId;
    // Additional data beyond the 48 bytes Mumble uses for identification
    uint32_t uiState;
    uint16_t compassWidth; // pixels
    uint16_t compassHeight; // pixels
    float compassRotation; // radians
    float playerX; // continentCoords
    float playerY; // continentCoords
    float mapCenterX; // continentCoords
    float mapCenterY; // continentCoords
    float mapScale;
    uint32_t processId;
    uint8_t mountIndex;
    uint32_t  floorId;       // NEW!
    uint32_t  sectorId;     // NEW!
};

Example

Example of v2/continents endpoint as reference of how quickly it becomes messy:

"https://api.guildwars2.com/v2/continents/<continentId>/floors/<floorId>/regions/<regionId>/maps/<mapId>/sectors?ids=all"

becomes

"https://api.guildwars2.com/v2/continents/1/floors/1/regions/4/maps/15/sectors?ids=all"

Anything else?

Ref: original PR#673

TheMrMilchmann commented 2 years ago

Fundamentally, I agree that this is a very useful feature to have but I'm not happy with the proposed structure for two reasons:

  1. The first 48 bytes of the context are used by Mumble to ensure that positional audio is delivered to the appropriate users. Adding additional fields here that are not constant within a map such as a sector ID breaks positional audio. Additionally, adding fields in the middle of the struct will break all existing MumbleLink reading tools for GW2. To retain compatibility and to not break positional audio, all fields should be added at the end of the struct.
  2. There is a hard limit on how much data can be exposed using MumbleLink. Specifically, the context struct has a 256 byte limit. While adding things like continentId is useful for developers as you demonstrated, I'd rather preserve as much of the free space as possible for future additions. Since continentId and regionId can be trivially inferred from the API, I propose only adding floorId and sectorId to the context. (Technically, sectorId can be calculated as well, but it can be fairly tricky since sectors can be arbitrary polygons.)
agaertner commented 2 years ago

Good point. Initially this was about floorId and sectorId anyway. I've put them around mapId to just highlight the hierarchy for the example. Definitely add them to the end.

agaertner commented 2 years ago

Add use case

  1. Conviniently trigger a request of sector constrained data from a backend. ( "paginating" map constrained data.)