YaLTeR / OpenAG

An open-source reimplementation of Adrenaline Gamer's client library.
https://j.mp/OpenAG
Other
131 stars 42 forks source link

Make a new list for hardcoded map series in Discord RPC module and fixed declaration of struct in com_model.h #185

Closed SmileyAG closed 8 months ago

SmileyAG commented 8 months ago

Closes #162

YaLTeR commented 8 months ago

Disabled by default, but can be enabled using discord_rpc_closest_map_match 1

What's the point? Nobody will know about this or use it. Is there any problem that prevents it from being always on? Any map name collisions?

SmileyAG commented 8 months ago

Disabled by default, but can be enabled using discord_rpc_closest_map_match 1

What's the point? Nobody will know about this or use it. Is there any problem that prevents it from being always on? Any map name collisions?

Let me give you a couple of examples of why this should be optional:

Example 1

Let's say we have a series of maps starting with hl1_bhop_uc1, say: hl1_bhop_uc1, hl1_bhop_uc1_beta1, hl1_bhop_uc1_beta2 and etc

For the most, these are the same maps, but with each version there a few differences, say in the placement / size of triggers or change of map geometry in a few places

So, imagine that in our list there will only be a thumbnail uploaded and filled only for hl1_bhop_uc1 (final version of map), but we will say that we are playing on a specific version of this series of map (hl1_bhop_uc1_beta5), therefore we will not have a thumbnail for it with the current code

This is where discord_rpc_closest_map_match 1 comes to the rescue And don’t need to tell me about the hardcode of the list of these series of maps, doing this would require to update client each time and also it took time for fill them

Example 2

Let's say that there is would be two maps: kz_synergy and kz_synergy_x, same in naming but completely different in map layout (one is more like bhop-stylish, other is kz-stylish)

Imagine that there is would be thumbnail only for kz_synergy, therefore it would also set a thumbnail for kz_synergy_x if discord_rpc_closest_map_match 1 is enabled, while in fact that are completely different map so it would be wrong to set it

YaLTeR commented 8 months ago

This is where discord_rpc_closest_map_match 1 comes to the rescue

Almost certainly nobody will bother to flip a cvar every time they happen to load a map with a problem like this (let alone know which maps are problematic).

Imagine that there is would be thumbnail only for kz_synergy, therefore it would also set a thumbnail for kz_synergy_x if discord_rpc_closest_map_match 1 is enabled, while in fact that are completely different map so it would be wrong to set it

Well, yeah. Is this going to be a common problem? If so then it might be better to not do this loose matching at all.

SmileyAG commented 8 months ago

Just you know, adding an thumbnail for each version of the same map series seems like a not smart decision in general, but there is also a issue with it, that in fact you can load a maximum of 300 images per API

Here I see only three options for how to generally solve this issue:

But in general, it’s not to say that there are just a lot of such map series, so maybe the idea of hardcoding this makes sense here

Just make a new list for which the code will also automatically perform a closest match search as it with discord_rpc_closest_map_match 1 enabled

Example:

// This list was specially created for a map series, let’s say that “hl1_bhop_am” series have a several versions of maps (example: `hl1_bhop_am_beta1`), but in fact those maps are almost identical
// If the beginning of map name matches with what presented in the list, then we set the same name for the thumbnail as in the list (e.g. map name: hl1_bhop_uc1_beta1, thumbnail name: hl1_bhop_uc1)
const std::unordered_set<std::string> map_series_with_thumbnails {
    "hl1_bhop_am"s,
    "hl1_bhop_bp1"s,
    "hl1_bhop_bp2"s,
    "hl1_bhop_faf"s,
    "hl1_bhop_lc"s,
    "hl1_bhop_oar"s,
    "hl1_bhop_ocwgh"s,
    "hl1_bhop_pu"s,
    "hl1_bhop_qe"s,
    "hl1_bhop_rp"s,
    "hl1_bhop_st"s,
    "hl1_bhop_uc1"s,
    "hl1_bhop_uc2"s,
};

First, in the code, we will check whether there is a map in the maps_with_thumbnails list, and if not, then we will check whether if the start of map name is matching with what in the map_series_with_thumbnails list (but don't do it in reverse order, otherwise the map that have a unique thumbnail will lose the opportunity to set it)

YaLTeR commented 8 months ago

Yeah, hardcoding like this sounds like a reasonable approach in this case. And it can be always enabled without a cvar.

YaLTeR commented 8 months ago

Thanks