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

AoE: Infobox player bugfixes #2577

Merged
merged 6 commits into from
Feb 15, 2023
Merged

AoE: Infobox player bugfixes #2577

merged 6 commits into from
Feb 15, 2023

Conversation

mbergen
Copy link
Collaborator

@mbergen mbergen commented Feb 14, 2023

Summary

Without resolved ID, querying would fail for lowercased IDs.
Removing the pagename as that doesn't work in userspace and ID is mandatory anyways.
Added optional query for pagenames with underscores instead of spaces.

Also changed the array merge to do one after another (otherwise, games might be added multiple times)

How did you test this change?

/dev

@hjpalpha
Copy link
Collaborator

since we want to store pagfenames with underscore in the future imo already prep the module for the switch and query for
person or person:gsub(' '; '_')

@mbergen mbergen requested a review from hjpalpha February 14, 2023 19:33
@hjpalpha
Copy link
Collaborator

hjpalpha commented Feb 14, 2023

i mean to actually have them as an OR condition not just replacing them
as atm they are still stored with space

see e.g. https://github.com/Liquipedia/Lua-Modules/blob/main/components/infobox/wikis/starcraft2/infobox_person_player_custom.lua
lines 359 -360

as reference: #1802

@mbergen
Copy link
Collaborator Author

mbergen commented Feb 14, 2023

Should i add another bugfix in here or a separate PR?
Edit: Added it now, can revert if needed

@mbergen mbergen changed the title AoE: Use resolved ID to query placements AoE: Infobox player bugfixes Feb 14, 2023
@mbergen mbergen requested a review from Rathoz February 14, 2023 21:12
@Rathoz Rathoz merged commit defe08d into main Feb 15, 2023
@Rathoz Rathoz deleted the aoe-infobox-player-fix branch February 15, 2023 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants