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.

Sign up for GitHub

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

Jump to bottom

LPA-6543: Customs that do resolve redirect on Player Pages must set underscore #1823

Merged
merged 2 commits into from
Sep 5, 2022
Merged

LPA-6543: Customs that do resolve redirect on Player Pages must set underscore #1823

merged 2 commits into from
Sep 5, 2022

Conversation

Rathoz
Copy link
Collaborator

@Rathoz Rathoz commented Sep 5, 2022 •
edited
Loading

Summary

Match2 wiki input customs, that resolve redirect of Player Pages must ensure that underscore instead of space is used.

Related to #1802, this one fixes Players in Teams in match2 only.

How did you test this change?

Tested with dev on dota2

@Rathoz Rathoz added c: match2 g: dota2 DOTA2 g: wildrift League of Legends: Wild Rift g: mobilelegends Mobile Legends: Bang Bang g: honorofkings Honor of Kings / Arena of Valor g: leagueoflegends League of Legends g: pokemon Pokemon Games labels Sep 5, 2022
@Rathoz Rathoz requested a review from hjpalpha September 5, 2022 13:06
@hjpalpha
Copy link
Collaborator

hjpalpha commented Sep 5, 2022

missing sc/sc2
https://liquipedia.net/commons/index.php?title=Module:MatchGroup/Input/Starcraft&action=edit#mw-ce-l497
https://liquipedia.net/commons/index.php?title=Module:MatchGroup/Input/Starcraft&action=edit#mw-ce-l520
https://liquipedia.net/commons/index.php?title=Module:MatchGroup/Input/Starcraft&action=edit#mw-ce-l525
https://liquipedia.net/commons/index.php?title=Module:MatchGroup/Input/Starcraft&action=edit#mw-ce-l577
https://liquipedia.net/commons/index.php?title=Module:MatchGroup/Input/Starcraft&action=edit#mw-ce-l639
https://liquipedia.net/commons/index.php?title=Module:MatchGroup/Input/Starcraft&action=edit#mw-ce-l1006

breaking change for sc/sc2 though (will break lots of queries)

@Rathoz
Copy link
Collaborator Author

Rathoz commented Sep 5, 2022

missing sc/sc2 https://liquipedia.net/commons/index.php?title=Module:MatchGroup/Input/Starcraft&action=edit#mw-ce-l497 https://liquipedia.net/commons/index.php?title=Module:MatchGroup/Input/Starcraft&action=edit#mw-ce-l520 https://liquipedia.net/commons/index.php?title=Module:MatchGroup/Input/Starcraft&action=edit#mw-ce-l525 https://liquipedia.net/commons/index.php?title=Module:MatchGroup/Input/Starcraft&action=edit#mw-ce-l577 https://liquipedia.net/commons/index.php?title=Module:MatchGroup/Input/Starcraft&action=edit#mw-ce-l639 https://liquipedia.net/commons/index.php?title=Module:MatchGroup/Input/Starcraft&action=edit#mw-ce-l1006

breaking change for sc/sc2 though (will break lots of queries)

Would only be the 639 (+ 669 may be needed to be force as SC2 TCs might store differently compared to other wikis) & maybe 1006. Only doing playters for opponentType team here.

However, sense it's less urgent on SC/SC2, and only SC/SC2 has significantly amount of dependencies towards this field, I think leaving it to be handled in the wider #1802 makes sense.

@hjpalpha
Copy link
Collaborator

hjpalpha commented Sep 5, 2022 •
edited
Loading

missing sc/sc2 https://liquipedia.net/commons/index.php?title=Module:MatchGroup/Input/Starcraft&action=edit#mw-ce-l497 https://liquipedia.net/commons/index.php?title=Module:MatchGroup/Input/Starcraft&action=edit#mw-ce-l520 https://liquipedia.net/commons/index.php?title=Module:MatchGroup/Input/Starcraft&action=edit#mw-ce-l525 https://liquipedia.net/commons/index.php?title=Module:MatchGroup/Input/Starcraft&action=edit#mw-ce-l577 https://liquipedia.net/commons/index.php?title=Module:MatchGroup/Input/Starcraft&action=edit#mw-ce-l639 https://liquipedia.net/commons/index.php?title=Module:MatchGroup/Input/Starcraft&action=edit#mw-ce-l1006
breaking change for sc/sc2 though (will break lots of queries)

Would only be the 639 (+ 669 may be needed to be force as SC2 TCs might store differently compared to other wikis) & maybe 1006. Only doing playters for opponentType team here.

However, sense it's less urgent on SC/SC2, and only SC/SC2 has significantly amount of dependencies towards this field, I think leaving it to be handled in the wider #1802 makes sense.

if it is changed it should be changed for all cases (per wiki)
else it will break [[player::...]] queries

hjpalpha
hjpalpha reviewed Sep 5, 2022
View reviewed changes
Copy link
Collaborator

@hjpalpha hjpalpha left a comment

Choose a reason for hiding this comment

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

please also test on aov, pokemon, wildrift and mobile legends, iirc they had some dependency further down in the modules when determining participants (in maps)

@Rathoz
Copy link
Collaborator Author

Rathoz commented Sep 5, 2022

missing sc/sc2 https://liquipedia.net/commons/index.php?title=Module:MatchGroup/Input/Starcraft&action=edit#mw-ce-l497 https://liquipedia.net/commons/index.php?title=Module:MatchGroup/Input/Starcraft&action=edit#mw-ce-l520 https://liquipedia.net/commons/index.php?title=Module:MatchGroup/Input/Starcraft&action=edit#mw-ce-l525 https://liquipedia.net/commons/index.php?title=Module:MatchGroup/Input/Starcraft&action=edit#mw-ce-l577 https://liquipedia.net/commons/index.php?title=Module:MatchGroup/Input/Starcraft&action=edit#mw-ce-l639 https://liquipedia.net/commons/index.php?title=Module:MatchGroup/Input/Starcraft&action=edit#mw-ce-l1006
breaking change for sc/sc2 though (will break lots of queries)

Would only be the 639 (+ 669 may be needed to be force as SC2 TCs might store differently compared to other wikis) & maybe 1006. Only doing playters for opponentType team here.
However, sense it's less urgent on SC/SC2, and only SC/SC2 has significantly amount of dependencies towards this field, I think leaving it to be handled in the wider #1802 makes sense.

if it is changed it should be changed for all cases (per wiki) else it will break [[player::...]] queries

Hence I think it's better to leave SC/SC2 for 1802. The other wikis don't have any match2 queries to the best of my knowledge.

@Rathoz
Copy link
Collaborator Author

Rathoz commented Sep 5, 2022 •
edited
Loading

please also test on aov, pokemon, wildrift and mobile legends, iirc they had some dependency further down in the modules when determining participants (in maps)

Should be correct now, but looked for pages using this functionality on any of these wikis and couldn't find (I looked at the latest S-Tier tournament and did insource search of t1p1)

@hjpalpha
Copy link
Collaborator

hjpalpha commented Sep 5, 2022

please also test on aov, pokemon, wildrift and mobile legends, iirc they had some dependency further down in the modules when determining participants (in maps)

Should be correct now, but looked for pages using this functionality on any of these wikis and couldn't find (I looked at the latest S-Tier tournament and did insource search of t1p1)

that might be true, but in case it will ever be used it should work^^

hjpalpha
hjpalpha approved these changes Sep 5, 2022
View reviewed changes
Copy link
Collaborator

@hjpalpha hjpalpha left a comment

Choose a reason for hiding this comment

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

LGTM

@Rathoz Rathoz merged commit 5fcfc96 into main Sep 5, 2022
@Rathoz Rathoz deleted the player-pagename-underscore-match2-customs branch September 5, 2022 14:04
Rathoz added a commit that referenced this pull request Oct 4, 2022
@Rathoz
LPA-6543: Customs that do resolve redirect on Player Pages must set u…
1d908a7 
…nderscore (#1823)

* Match2: Wiki customs that do resolve redirect on Player Pages must set underscore

* Fix attach
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hjpalpha hjpalpha hjpalpha approved these changes

Assignees
No one assigned
Labels
c: match2 g: dota2 DOTA2 g: honorofkings Honor of Kings / Arena of Valor g: leagueoflegends League of Legends g: mobilelegends Mobile Legends: Bang Bang g: pokemon Pokemon Games g: wildrift League of Legends: Wild Rift
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Rathoz @hjpalpha