Skip to content

Implement AP_SetTags - #45

Open
SilicDev wants to merge 1 commit into
N00byKing:mainfrom
SilicDev:set_tags
Open

Implement AP_SetTags#45
SilicDev wants to merge 1 commit into
N00byKing:mainfrom
SilicDev:set_tags

Conversation

@SilicDev

Copy link
Copy Markdown

Implements a function to update server connection tags.

Many client protocols like DamageLink and RingLink as well as DamageLinkGroups rely on setting tags to receive bounce packets. As it is currently, without modifications the API is limited to protocols directly supported by the APCpp library. The aim of this PR is to add the option to handle these in the actual client code.

For this purpose the function also modifies the local room info tags so code that changes these tags can be aware of which tags are already added and does not override them unintentionally as AP doesn't send a proper response packet to update the local state.

Comment thread Archipelago.cpp
void AP_SetTags(std::vector<std::string> const& tags)
{
lib_room_info.tags = tags;
if (!multiworld) return;

@N00byKing N00byKing Aug 14, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this to the top of the function.
Also, the function should probably be guarded by auth, since we don't want a ConnectUpdate before Connect

EDIT: On the same note, might make sense to call it "UpdateTags" instead then

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the updating of the tags is specifically not below the guard so that the function acts locally the same no matter if connected to a server or not, so that client code checking for the existence of these tags doesn't need to be aware if a server exists or not, I'm not even sure it can be aware of that as is right now.
An auth guard wasn't included as no other function but APSend implements one.

I can still implement these changes, just want to make sure that they are desired given the reasoning behind them.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, thats a good point. Better to leave it at that, just rename the function.

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