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

feat(ClanWar): add getMemberByMapPosition #189

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ryancundiff
Copy link

Add method to ClanWar to provide encapsulated functionality for getting a member by their map position in war.

Add method to ClanWar to provide encapsulated functionality for getting a member by their map position in war.
@csuvajit
Copy link
Contributor

can you please bump the version as well?

@ryancundiff
Copy link
Author

can you please bump the version as well?

Done!

@csuvajit
Copy link
Contributor

I would like to bring to your attention a slight inaccuracy in the mapPosition for CWL. When the team size is 30, but all 50 members were registered during the signup process, the mapPosition value exceeds 30.

@csuvajit
Copy link
Contributor

The mapPosition is based on the registered member list for CWL

If you do getMemberByMapPosition(10), it might return a wrong member or just null (if they were excluded from that round)

@ryancundiff
Copy link
Author

ryancundiff commented May 22, 2024

The mapPosition is based on the registered member list for CWL

If you do getMemberByMapPosition(10), it might return a wrong member or just null (if they were excluded from that round)

How do you recommend I work around that? If it can return the wrong member then there is no way (that I can think of) to work around that. I could remove the functionality for CWL by checking the ClanWar type and return null if it is equal to CWL. What do you think?

@csuvajit
Copy link
Contributor

If you sort them by the mapPosition, the index + 1 becomes the actual mapPosition.

@csuvajit
Copy link
Contributor

However, I'm not sure if the members are already sorted by the mapPosition.
I can comfirm that in 2-3 hours.

@ryancundiff
Copy link
Author

However, I'm not sure if the members are already sorted by the mapPosition. I can comfirm that in 2-3 hours.

Did you confirm?

@csuvajit
Copy link
Contributor

It is not sorted.

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.

None yet

2 participants