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

[WIP] Tournament badges 212 #321

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

[WIP] Tournament badges 212 #321

wants to merge 3 commits into from

Conversation

jj-apps
Copy link
Contributor

@jj-apps jj-apps commented Dec 31, 2020

#212 Front end implementation of tournament badges

image
image

@jj-apps
Copy link
Contributor Author

jj-apps commented Dec 31, 2020

@Cepheid-UK @abeninski

Here's the front-end implementation of the tournament badges, but I need some guidance on how the backend should be built.

@modmoto
Copy link
Member

modmoto commented Jan 20, 2021

@jj-apps
hey, I just saw that you searched for some guidance, 20 days ago ^^

What can I help you with? Did you built the backend allready or are you unfamiliar with .net?

@modmoto
Copy link
Member

modmoto commented Jan 20, 2021

I just realized in the backend, that we are having an issue with the tournament, that we should fix first. The tournament does not support battletags, just plain strings. So before we add the tournament badges, we should add an auto completion feature for the tournament badge and save the battletags within the tournament. Then we could update the player profile for the tournament aswell. And then the ui should work out of the box.

Like in the backend, the class TournamentPlayer should support a field BattleTag . And then we can join this class with the player profile class when someone opens a page, for example.

@jj-apps
Copy link
Contributor Author

jj-apps commented Jan 20, 2021

@jj-apps
hey, I just saw that you searched for some guidance, 20 days ago ^^

What can I help you with? Did you built the backend allready or are you unfamiliar with .net?

@modmoto I'm familiar with .Net and can do the backend implementation for this. Just need guidance on where in the database this should be stored (first time using MongoDB for me).

Would adding BattleTag to TournamentPlayer be all we need? Then it can be returned via a join with the player profile?

I suppose we would need to update the Tournament admin UI to allow for entering the btag too.

@modmoto
Copy link
Member

modmoto commented Jan 21, 2021

jeah, updating the admin UI would be needed for that. But I would make it just a tiny bit more complex, in making the name field for the player a search box, like on the ranking page, that returns the small profile with name and btag. Then we can just take this battletag and name from the backend. This makes the tournament page much easier to handle and on top off that we can store the correct battletag in the backend. So we dont have to go through the old tournaments and searching the battletag by hand. I know this is a "little" more work than expected, but ultimately we wanted to do the search like that anyways and as we need that info, I think this is a good opportunity to go for it.

Regarding the mongodb:
Joining is a little weird, we are doing this on a few occasions but not very often. Like here for example: https://github.com/w3champions/website-backend/blob/master/W3ChampionsStatisticService/PersonalSettings/PersonalSettingsRepository.cs
The important thing is the Lookup<T>. The dto has to have a list of some sort, where you join the data into. In a first and easy way, we could just load the tournaments when returning the profiles and do the join on our own. Performance wise, that is ok, as the whole DB is in Memory anyways. Codes wise it is a little rubish. But we have it like that for more complicated joins, for example profile and rankings. Another easy way would be to create a new collection with TournamentResults where you have an object like that (exactly like the one that you wanted in the participatedInTournament property

{
  "battleTag": "peter#123",
  "participatedInTournaments": [
    {
       "id": "tour1"
       "place": 1
    },
    {
      "id": "tour2"
      "place": 3
    },
  ]
}

This can be easily written and more importantly we dont have to join. Like it is intended with mongo. We just load this, when someone opens a profile page and put it on the dto where you wanted it in this PR. We could also just make a separate route, to keep the objects separated (I would actually prefer that). Something like api/tournament-results/{id}. What I would not do, in any way, is to update the profile model with the tournament results. Because the profile is a readmodel and gets updated after every game. This means we can run into race conditions. To be fair, that will most likely not happen, because after we report a tournament result, the players are most likely not playing, but it is still a race condition that I would like to avoid. Keep the readmodels a read only collection for the syncers.
Regarding technical stuff: There is a base repository in the backend that makes read and writes a little bit easier. But the MongoClient class is also not very complicated to use. When a collection is not present and you write to it, it gets generated automatically, so you dont have to have access to the db server for that. We have a open test db, that you can use (connectionstring is in the repo). I would recommend Mongodb Compass as a tool to access the db quickly and see if your data was stored right.

@modmoto
Copy link
Member

modmoto commented Jan 21, 2021

Just had a shower and scratch that idea with the additional collection. We can just add a route api/tournament-results/{id} that iterates over the torunaments and generates that dto. But we still need the battletag for that. I will make the API accept a battletag in the dto and add the route in a separate PR. Then you can focus on the UI for that.

@abeninski
Copy link
Contributor

I think after @gabcinder2004 tournaments get merged the winners for the next round will be auto determined so no more manual action from admin and no need to write the code to link battle tags?

@modmoto
Copy link
Member

modmoto commented Jan 21, 2021

But we still need battletags, right? Right now we only enter names and dont link them to the account.

@abeninski
Copy link
Contributor

abeninski commented Jan 21, 2021

People register from ingame and their battle tag is used. Those tourneys should be generated and controled automatically by matchmaking

@modmoto
Copy link
Member

modmoto commented Jan 21, 2021

Also our season finals? Because this PR is about that.

@abeninski
Copy link
Contributor

Yes. Season finals should use Gab's tournaments as well. I don't see a reason why not use it?

@modmoto
Copy link
Member

modmoto commented Jan 21, 2021

yes, good idea. I was just wondering if this might be an issue with observers and stuff like that. Or if b2w and the other streamers ar getting this.

@Cepheid-UK
Copy link
Member

Cepheid-UK commented Nov 14, 2021

@jj-apps to say we have neglected PRs is an understatement seeing as this is now 11 months old 😄 but we're trying to get back on track.

This is still very relevant, we're going to be adding the ability for admins to give players badges, and the automated tournaments are still going to be using these too, somehow, not sure exactly yet.

do you want to update this and we can work on a backend solution, even if its only temporary or manually done.

@PlayLikeNeverB4 PlayLikeNeverB4 changed the title Tournament badges 212 [WIP] Tournament badges 212 Sep 15, 2022
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.

4 participants