kmph
2018-10-13 14:28

Oficjalna dokumentacja do .Net zaczyna mnie irytować. Sam nie wiem, czy oni piszą nieściśle, omijając istotne informacje, czy ja jestem za głupi, by dokumentację poprawnie zrozumieć.

4 przykłady z ostatnich dni tylko.

System.Net.IPAddress.GetAddressBytes

Provides a copy of the IPAddress as an array of bytes.

title

Jestem w szoku, Szerloku. Naprawdę? No nie spodziewałbym się, cóż innego mogłoby robić public byte[] GetAddressBytes?

A tak poważnie, to ja wszedłem na tę stronę dokumentacji, by dowiedzieć się czegoś zupełnie innego: Jaka jest kolejność bajtów? Network order? Host order? Oczywiście da się to w miarę łatwo po prostu sprawdzić, wypisując sobie na konsoli zwrócone bajty z jakiegoś prostego adresu np. ::1, ale (a) po to jest dokumentacja, by nie trzeba było sprawdzać, (b) sprawdzanie ma ograniczoną niezawodność - choć to mało prawdopodobne, ale co jeżeli np. zwraca bajty w host order jeśli obiekt IPAddress powstał przez sparsowanie podanego stringa, a w network order, jeśli sprawdzamy źródło żądania?

System.Tuple<T1, T2>.IComparable.CompareTo(Object)

Compares the current Tuple<T1,T2> object to a specified object and returns an integer that indicates whether the current object is before, after, or in the same position as the specified object in the sort order.

No shit Sherlock! Ech, jeden obrazek Kapitana starczy, drugiego nie będę wklejał.

Wchodząc na tę stronę dokumentacji chciałem się upewnić, czy elementy krotki są porównywane w porządku leksykograficznym, czy jakoś inaczej. Operator <= nie jest zdefiniowany dla Tuple, by użyć powyższej metody trzeba zcastować do IComparable, to wzbudziło moje podejrzenia, czy nie dzieje się coś dziwnego.

Co prawda tym razem na tej stonie doku jest podany przykład, z którego zdaje się wynikać, że jest to po prostu porządek leksykograficzny... ale nie wiem, wolałem zaimplementować porównywanie samodzielnie. Może to pozostałość z C++... ale jak mi się wyraźnie nie powie w doku, że mi coś wolno robić, to wolę nie robić, bo boję się demonów z nosa.

Microsoft.EntityFrameworkCore.DbContext.AddAsync

Returns

Task<entityentry>

A task that represents the asynchronous Add operation. The task result contains the EntityEntry for the entity. The entry provides access to change tracking information and operations for the entity.

Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync

Returns
Task<int32>

A task that represents the asynchronous save operation. The task result contains the number of state entries written to the database.

Exceptions
DbUpdateException

An error is encountered while saving to the database.

To, co chciałem wiedzieć, to co się stanie, jeśli spróbuje się wsadzić wiersz o id identycznym z już istniejącym. Z tego, co napisane powyżej, mam 3 hipotezy: (a) EntityEntry zwrócona przez AddAsync zawiera jakieś pole / property, która informuje o takiej okoliczności; (b) SaveChangesAsync zwraca 0 i nic innego się nie dzieje; (c) SaveChangesAsync rzuca DbUpdateException. Znów musiałem uruchamiać pod debuggerem i sprawdzić, że (c).

Microsoft.EntityFrameworkCore.DbContext.FindAsync<tentity>(Object[])

Parameters

keyValues Object[]

The values of the primary key for the entity to be found.

Co co co co co? Dobra załóżmy że mam kompozytowy klucz główny, składający się np. z dwóch stringów (imię i nazwisko). To teraz jak mam podać te parametry metodzie FindAsync, żeby odp. argumenty przyporządkowała odpowiednim polom? Ja chcę znaleźć Marka (imię) Jurka (nazwisko), nie zaś kogoś o imieniu Jurek i nazwisku Marek. Czy kolejność jest istotna? Czy też może metoda może znaleźć któregokolwiek z tych dwóch, i nie ma się na to wpływu?

OK z tym imieniem i nazwiskiem to wymyśliłem (bo nie można zakładać, że kombinacja imię+nazwisko się nie powtarza), ale prawdziwy use case to jest zapisywanie adresów IPv6 jako krotki dwóch ulongów, bo niestety nie ma liczby 128-bitowej. Nie chcę zapisywać dwóch tych samych adresów w bazie, logiczne wydaje mi się więc połączenie obu ulongów jako primary key (bo semantycznie to one właśnie identyfikują wiersz przecież).

Nie mogę wyrozumieć się w doku... trudno. Where + FirstOrDefault rozwiąze problem.

#mon

AreQrm

Możesz połączyć to w samo FirstOrDefault ;-)

kmph
2018-10-09 00:29

Z powodu długości zmuszony byłem podzielić ten wpis na mikroblogu na 2. Zatem kolejne trudności z tego commita #mon :

Na tym fragmencie kodu naciąłem się jeszcze kilka razy. Po pierwsze i najprostsze: Zazwyczaj wyjątki zwalają tylko pojedynczy request... jednak jeśli wyjątek poleci w handlerze timera, to... CAŁY SERWER LECI W DÓŁ. Tak, cały serwer się crashuje. W związku z tym:

                timer.Elapsed += async (sender, e) =>
                {
                        try
                        {
                            using (var scope = scopeFactory.CreateScope())
                            {
                                var Clients = scope.ServiceProvider.GetRequiredService<IHubContext<ChatHub>>().Clients;
                                var userManager = scope.ServiceProvider.GetRequiredService<UserManager<ApplicationUser>>();
 
                                await userManager.RemoveFromRoleAsync(user, "Muted"));
                                var expirationMessage = $"Mute of {message.author.UserName} has expired.";
                                await Clients.All.SendAsync("ReceiveChatMessage", expirationMessage);
                            }
                            catch {} // Tu wypadało by przynajmniej zalogować wyjątek
                        }
                 }

I drugi problem. Powyższe jest uproszczeniem: mute'y działają raczej w ten sposób, że mod klika sobie na wiadomość w czacie i tam wyświetla mu się admińskie menu. Wobec tego kod wyglądał raczej mniej więcej tak:

        [Authorize]
        public async Task Mute(long chatMessageId)
        {
            var message = await dbContext.ChatMessages.AsNoTracking().Include(ch => ch.author).FirstOrDefaultAsync(ch => ch.Id == chatMessageId);
            if(message == null) return;
 
            if ((await userManager.AddToRoleAsync(message.author, "Muted")).Succeeded)
            {
                var muteMessage = $"{message.author.UserName} has been muted.";
                await Clients.All.SendAsync("ReceiveChatMessage", muteMessage);
 
                var timer = new Timer () { Interval = 2 * 60 * 60 * 1000, AutoReset = false };
                timer.Elapsed += async (sender, e) =>
                {
                    await userManager.RemoveFromRoleAsync(message.author, "Muted"));
                    var expirationMessage = $"Mute of {message.author.UserName} has expired.";
                    await Clients.All.SendAsync("ReceiveChatMessage", expirationMessage);
                };
                timer.Start();
            }
        }

Kod wyglądął mniej więcej tak, wyjąwszy, że niektóre fragmenty były wydzielone do osobnych metod.

Taki kod rzucał wyjątkami (znów...) Otóż Entity Framework się rzucał, że coś tam z trackowaniem było źle. Wyjątek leciał przy próbie dodania usera do roli.

Wyrzucenie AsNoTracking z linii zaciągającej message pomogło... wyjątek leciał dopiero przy wyjmowaniu usera z roli (postęp, nie?)

Rozwiązaniem okazało się... coś takiego:

        private async Task<bool> AddToRole(string userId, string role)
        {
            var user = await userManager.FindByIdAsync(userId);
            if (user == null) return false;
 
            var res = await userManager.AddToRoleAsync(user, role);
            if (res.Succeeded)
            {
                await SendRoleUpdate(user);
                return true;
            }
            else return false;
        }
 
        // analogicznie RemoveFromRole
 
        [Authorize]
        public async Task Mute(long chatMessageId)
        {
            var message = await dbContext.ChatMessages.AsNoTracking().Include(ch => ch.author).FirstOrDefaultAsync(ch => ch.Id == chatMessageId);
            if(message == null) return;
 
            if (await AddToRole(message.authorId, "Muted"))
            {
                var muteMessage = $"{message.author.UserName} has been muted.";
                await Clients.All.SendAsync("ReceiveChatMessage", muteMessage);
 
                var timer = new Timer () { Interval = 2 * 60 * 60 * 1000, AutoReset = false };
                timer.Elapsed += async (sender, e) =>
                {
                    await RemoveFromRole(message.authorId, "Muted"));
                    var expirationMessage = $"Mute of {message.author.UserName} has expired.";
                    await Clients.All.SendAsync("ReceiveChatMessage", expirationMessage);
                };
                timer.Start();
            }
        }

Działa. (Chyba). Nie wiem, o co chodziło.

I wreszcie ostatnia sprawa: banowanie po IP. No więc po pierwsze: z moich doświadczeń wynika, że banowanie po zakresach IP potrafi być niezbędne, gdy spamer skacze sobie np. po adresach IP Neostrady. Co prawda framework daje klasę IPAddress, no ale ona nie obsługuje zakresów... Na szczęście mamy adekwatną paczkę w NuGetach: IPAddressRange

No to myślę sobie, teraz implementacja banowania będzie prosta. Daję sobie tabelkę do bazy danych, jeden wiersz = jeden zakres, i jedziemy:

        // model do db
       public class IPBans
       {
           [Key] public long Id { get; set; }
           public string IPRangeStr { get; set; }
           [NotMapped] public IPAddressRange IPRange => IPAddressRange.Parse(IPRangeStr);
       }
 
        // metoda w ChatHubie
        private async Task<bool> IsIPBanned(IPAddress ipaddr)
        {
            return await dbContext.IPBans.Where(ban => ban.IPRange.Contains(ipaddr).AnyAsync();
        }

...wystarczy? Znowu nie! Wyjątki! Poprawiam się:

        private async Task<bool> IsIPBanned(IPAddress ipaddr)
        {
            return await dbContext.IPBans.Where(ban => IPAddressRange.Parse(ban.ip).Contains(ipaddr).AnyAsync();
        }

I dalej wyjątki. Ponownie się poprawiam:

        private async Task<bool> IsIPBanned(IPAddress ipaddr)
        {
            return (await dbContext.ipbans.ToListAsync()).Where(ban => ban.ip.Contains(ipaddr)).Any();
        }

W zasadzie to zrozumiałe. Czego ja oczekiwałem? Że baza danych zna semantykę nieoficjalnej biblioteki? Nie no, jasne jest, że to musiało byc wykonane na serwerze, nie na bazie.

Nie wygląda to na najlepszy performance, gdyby się chciało masowo i automatycznie banować tory, proxy i takie tam. Na szczęście składnia zakresów adresów IP, nawet jeśli zapisana tylko w formie stringa, nie jest taka trudna i gdyby przysiąść, dałoby się (mam nadzieję) zaimplementować to przeszukiwanie na bazie danych tak, by czas był logarytmiczny, a nie wymagał zaciągania wszystkich banów.

Tylko że to upierdliwe będzie. I dołączenie biblioteki okaże się dalece mniej użyteczne, niż powinno. Znów zaczynam odnosić wrażenie, że coś źle robię... bo nie wyobrażam sobie, żeby porządne banowanie po zakresach nie było problemem rozwiązanym i wymagało ręcznej implementacji przeszukiwania adresów.

kmph

@Afish CO do IP: niestety IPv6 to 128bit, oczywiście da się to łatwo zaimplementować i porównywać dwoma 64bitowymi liczbami, jednak zastanawiam się czy gdyby zapisać adres IP w pełnej postaci w stringu to by porównanie alfabetyczne nie zadziałało od razu

Afish

Zależy od collation bazy danych.

kmph
2018-10-09 00:28

Po absurdalnie długim czasie wreszcie nowy commit w repo #mon .

W zasadzie powinienem się raczej uczyć teraz, no ale cóż, raz na tydzień można coś poddłubać, nadrobię w najbliższych dniach. Tak wiem, z organizacją czasu u mnie kiepsko... Jest to moja poważna słabość, muszę starać się to naprawić.

No ale nie o tym miałem. Zaprototypowałem więc podstawową funkcjonalność banowania. Tak po kontach, jak i po IP.

Są 2 rodzaje kar: mute i ban. Mute się sam zdejmuje po 2h, ban trwa aż do ręcznego zdjęcia, to cała różnica. Przy czym ban na konto to z automatu mute na IP. Mute na konto to także z automatu mute na IP.

Założenie mute ZAWSZE próbuje zdjąć blokadę po 2h. To znaczy, jeśli się najpierw założy mute, a potem ręcznie dorobi bana, to i tak nie ma to znaczenia, bo po 2h mute i tak odblokuje konto lub IP. Tak samo, jeśli założy się mute na bana, to po 2h mute tego bana zdejmie. I wreszcie rzecz najgorsza: Jeśli się założy mute, zdejmie mute ręcznie i przed upływem 2h założy bana... to ten zdjęty ręcznie mute po upływie 2h od jego założenia i tak tego bana zdejmie. Zdaję sobie sprawę, że to jest zbyt prymitywne w ogólnym zastosowaniu, ale na razie musi wystarczyć.

Jak pisałem wcześniej, tego rodzaju czasowe zabawy, nie widząc lepszej alternatywy, implementowałem za pomocą System.Timers.Timer. Tutaj jednak od razu pojawił się poważny problem. Otóż tego rodzaju kod nie działa:

    public class ChatHub : Hub
    {
        private readonly ApplicationDbContext dbContext;
        private readonly UserManager<ApplicationUser> userManager;
        public ChatHub(ApplicationDbContext dbContext, UserManager<ApplicationUser> userManager)
        {
            this.dbContext = dbContext;
            this.userManager = userManager;
        }
 
        [Authorize]
        public async Task Mute(string username)
        {
            var user = await userManager.FindByNameAsync(username);
            if(user == null) return;
 
            if ((await userManager.AddToRoleAsync(user, "Muted")).Succeeded)
            {
                var muteMessage = $"{user.UserName} has been muted.";
                await Clients.All.SendAsync("ReceiveChatMessage", muteMessage);
 
                var timer = new Timer () { Interval = 2 * 60 * 60 * 1000, AutoReset = false };
                timer.Elapsed += async (sender, e) =>
                {
                    await userManager.RemoveFromRoleAsync(user, "Muted"));
                    var expirationMessage = $"Mute of {user.UserName} has expired.";
                    await Clients.All.SendAsync("ReceiveChatMessage", expirationMessage);
                };
                timer.Start();
            }
        }
    }

Kod ten rzuca wyjątkami. Podług mojego rozumienia winny tu jest sposób, w jaki framework obsługuje dependency injection. W chwili, gdy metoda Mute zwraca, framework Dispose'uje wszystkie parametry konstruktora ChatHuba, w tym w szczególności userManagera, dbContext, no i Clients też, o ile dobrze pamiętam, były nie do użycia.

Myślę sobie, cóż; muszę się uciec do service locator pattern w handlerze timera. Ale jak?

Ichnia strona dokumentacji wskazywała wówczas, jakobym mógł zażyczyć sobie całego huba SignalR. Do układanki dołożyłem sobie info ze StackOverflow, że IServiceScopeFactory to singleton, więc jeśli zażądam go w konstruktorze huba, to mi go dependency injection nie zje przed uruchomieniem się handlera timera. Tej informacji nie mogłem znaleźć w dokumentacji, ale rzeczywiście IServiceScopeFactory ZDAWAŁ się działać tak, jak Stack napisał. Założyłem więc, że tak jest - nie wiem jednak, czy na pewno poprawnie.

Mój kod wyglądał więc mniej więcej tak:

                timer.Elapsed += async (sender, e) =>
                {
                    using (var scope = scopeFactory.CreateScope())
                   {
                       var hub = ActivatorUtilities.CreateInstance<ChatHub>(scope.ServiceProvider);
                      await hub.userManager.RemoveFromRoleAsync(user, "Muted"));
                      var expirationMessage = $"Mute of {message.author.UserName} has expired.";
                      await hub.Clients.All.SendAsync("ReceiveChatMessage", expirationMessage);
                    }
                };
                timer.Start();

Problem w tym, że ten kod... nie działał. Rzucał znowu wyjątkami.

Problem zdaje się być w dokumentacji, więc idę z tym znowu do devów. Dowiedziałem się, że: (a) Powinienem wyrzucić logikę z huba do osobnej klasy (Wiem! Ale nawet, jak to zrobię, to problem pozostanie dokładnie ten sam!), (b) By design nie wolno mi żądać instancji huba (CO? To jak ma to zaimplementować? Jestem bliski paniki); (c) Devy nie rozumieją, na czym polega problem, chcą code samples jednak (d) kiedy dałem code samples, nie odpowiedzieli. W sumie, zrozumiałe - nie są tam od tego, by debugować mój kod. Mimo to, biorąc pod uwagę, że SAMI POPROSILI o samples, czuję się niemile potraktowany. (może to moja wina, może samples to nie miały być wyrywki z mojego kodu, a Minimal, Complete, Verfiable example? Nawet myślałem, by takowy napisać, ale stwierdziłem, że musiałby być on tak duży, że bardziej będę zrozumiały, jeśli dam urywki z mojego kodu. Cóż, pewnie popełniłem błąd.

Ale! devy wspomniały coś o HubContext. To zaowocowało rozwiązaniem, które zdaje się działać, chociaż czy jest poprawne i czy aby tylko przypadkiem nie działa to doprawdy nie mam pojęcia:

                timer.Elapsed += async (sender, e) =>
                {
                        using (var scope = scopeFactory.CreateScope())
                        {
                            var Clients = scope.ServiceProvider.GetRequiredService<IHubContext<ChatHub>>().Clients;
                            var userManager = scope.ServiceProvider.GetRequiredService<UserManager<ApplicationUser>>();
 
                            await userManager.RemoveFromRoleAsync(user, "Muted"));
                            var expirationMessage = $"Mute of {message.author.UserName} has expired.";
                            await Clients.All.SendAsync("ReceiveChatMessage", expirationMessage);
                        }
                 }

Być moze to już zbytnia upierdliwość z mojej strony, ale zapytałem devów przed chwilą o to jeszcze raz, już tym razem ostatni.

kmph

@Afish Ciekawe, jakoś nie znalazłem tego pytania. Przeczytam. Dzięki za linka.

kmph
2018-09-14 00:34

Kolejny wpis w temacie #mon .

Długo nie robiłem żadnych commitów. Zatrzymał mnie problem banowania. Naprawdę sądziłem, że to będzie coś koło 1 dnia roboty. Jednak wąskim gardłem okazał się mój brak wiedzy, jak działa framework.

Problem wydaje się być raczej prosty: jest chatbox w tej grze, chcę dać modom możliwość uciszania kont, które spamują chatbox głupotami. W tym celu przeczytałem odnośny tutorial z oficjalnej dokumentacji: Create an ASP.NET Core app with user data protected by authorization Otóż proponują tam, by do zadań związanych z uprawnieniami wykorzystać mechanizm ról. No to świetnie: daję role Mod oraz Muted, Mod może sobie wołać na ChatHubie metodę Mute, w której jest tego rodzaju:

var bannedUser = await userManager.FindByNameAsync(username);
if(bannedUser != null)
{
    await userManager.AddToRoleAsync(bannedUser, "Muted");
}

Natomiast metoda SendMessage jest obwarowana kodem podobnym do tego:

if(Context.User.IsInRole("Muted")
    return;

(Tak - to jest dużo prostszy kod niż ten, który proponowali w tutorialu. Po prostu kod z tutoriala, wraz z rejestrowaniem Authorization Handlers etc, strzelaniem z armaty do wróbla. Może jednak ichni kod jest bardziej "czysty", tym bardziej, że w przeciwieństwie do mojego nie używają tam magic stringów.)

Z powyższym kodem jest jeden bardzo poważny problem: nie działa. Mod sobie banuje, zbanowany dalej może pisać.

Na szczęście pisząc ten kod, miałem już za sobą napisaną obsługę "kont anonimowych". Dzięki temu wiedziałem, czemu to może nie działać. Inaczej chyba nie wiedziałbym, gdzie szukać problemu.

O co chodzi z tymi kontami anonimowymi? Ano spodobał mi się mechanizm z pokemonshowdown.com, gdzie nie trzeba się rejestrować, by móc grać: trzeba jedynie wybrać sobie nicka. Wtedy to konto jest "niezerejestrowane", każdy może przejąć tego nicka, ale bitwy są zapisywane w historii tego konta. Ale w każdej chwili można sobie nicka zarejestrować, dając mu własne hasło. Brak konieczności rejestracji mi się spodobał, znacznie obniża to barierę wejścia.

No to próbowałem sobie zaimplementować to. Również za pomocą ról. Gdy ktoś wybiera sobie nicka, tworzone jest konto o tej nazwie. Konto nie ma hasła. Gdy ktoś rejestruje to konto (zakłada mu hasło), wówczas wołamy:

var user = await _userManager.GetUserAsync(User);
var result = await _userManager.AddPasswordAsync(user, Input.Password);
if(result.Succeeded)
    await _userManager.AddToRoleAsync(user, "Registered")

I od tej pory czat przestanie tego nicka wyświetlać kursywą, zaś kto będzie chciał sobie wybrać tego nicka, ten będzie musiał podać hasło. W myśl wyżej wymienionego tutoriala, za każdym razem, gdy chcemy wiedzieć, czy konto jest zarejestrowane, czy nie (w tym na stronie logowania), wołamy User.IsInRole("Registered")

No i znowu - ten kod zdawał się nie działać. Rejestrowałem konto, a ono dalej było traktowane jako niezarejestrowane. User.IsInRole("Registered") zwracał false.

Zauważyłem jednak, że już wylogowanie się z dopiero co zarejestrowanego konta i zalogowanie się na nie ponownie naprawia problem. Wtedy konto jest traktowane jako zarejestrowane i User.IsInRole("Registered") zwraca true.

Naprawiłem więc kod rejestrujący konto, zdaje się on działać po tych zmianach:

var user = await _userManager.GetUserAsync(User);
var result = await _userManager.AddPasswordAsync(user, Input.Password);
if (result.Succeeded)
{
    var resultRole = await _userManager.AddToRoleAsync(user, "Registered");
    if(resultRole.Succeeded)
    {
        await _signInManager.SignOutAsync();
        await _signInManager.SignInAsync(user, false);
    }
}

Przypuszczałem więc, że z tym banowaniem jest podobnie. I rzeczywiście - gdy tylko wyloguję się ze zbanowanego konta i zaloguję się na nie ponownie, konto zaczyna być wreszcie traktowane jako uciszone i nie może wypowiadać się w czacie.

Ale tu już miałem poważniejszy problem. Nie widziałem w SignInManager metody, która pozwalałaby wylogować innego usera. Wołanie, tak jak w kodzie odpowiadającym za rejestrację signInManager.SignOutAsync() spowoduje wylogowanie się moda, a nie zbanowanego usera.

Pytanie na Stacku. Nawet pojawiły się 3 odpowiedzi. A więc:

  1. (Zdownvote'owana, nie przeze mnie): Na siłę wylogować zbanowanego usera. No chyba jednak nie tak to powinno działać, mimo wszystko. Ani Wikipedia, ani pokemonshowdown, nie wologowują na siłę userów, którym odebrany został dostęp do jakiejś cześci serwisu.
    2.: Link do odpowiedzi na inne pytanie. Problemy z tą odpowiedzia są 3: (a) Jest z 14 roku, a zdaje mi się, że technologie około.netowe mają zwyczaj robić breaking changes w publicznych API; (b) Jak sądzę, zakłada, że operujemy na tym samym userze, który wysyła żądanie - a więc o ile mod nie banuje siebie samego, to nic nie rozwiązuje; (c) Jeśli jednak (b) nie zachodzi, to odpowiedź ta robi coś dziwnego, znalezienie dokumentacji do tych zabaw może być niełatwe, a wolałbym nie robić naprawdę dziwnych rzeczy bez znalezienia ich w dokumentacji;
    3.: (odpowiedź pojawiła się później): Że separation of concerns, że authorization to nie authentication, że roles to ani jedno, ani drugie - no dobrze, ale w takim razie co zrobić?

Udałem się z tym do devów .NET Core. Chwała im za to, że są skłonni odpowiadać na pytania userów - więc jako ostateczność takie rozwiązanie może pozostać. A tu miałem jak "zahaczyć" - zgłosiłem issue do tego tutoriala dokumentacji, który pisał o IsInRole. Odpowiedź ich była taka, że należy użyć signinManager.RefreshSignIn. Tak też uczyniłem:

var bannedUser = await userManager.FindByNameAsync(username);
if(bannedUser != null)
{
    await userManager.AddToRoleAsync(bannedUser, "Muted");
    await signInManager.RefreshSignInAsync(bannedUser);
}

I... Znowu nie działa! Kiedy wołam w hubie SignalR, rzuca wyjątkami. Kiedy wołam na RazorPage, wydaje się nie mieć żadnego efektu. Ale moja issue, którą otworzyłem celem tego pytania, jest już zamknięta.

Zaczynam bać się nadużycia cierpliwości devów. No dobrze, muszę im zaprzeczyć, ale zrobić to tak, by nie zostać zignorowanym. Trzeba by mieć dobre argumenty. Tworzę prościutki projekt, który ma dowieść mojego punktu widzenia i otwieram kolejny issue. Tym razem wreszcie mam odpowiedź: Nie wołać User.IsInRole("Muted") (tj. ClaimsPrincipal.IsInRole), bo to będzie zwracać nieaktualne dane i devy nie mają zamiaru nic z tym robić, tylko wołać UserManager.IsInRoleAsync("Muted"). A zatem poprawiony (i wreszcie działający!) kod sprawdzania, czy user jest uciszony:

var user = await userManager.GetUserAsync(Context.User);
 if ((await userManager.GetRolesAsync(user)).Contains("Muted")) return;

UFF! Tyle czasu na coś tak (zdawałoby się) prostego! Zaczynam być rozeźlony.

A przede mną kolejne problemy, których rozwiązanie, obawiam się, może przebiegać podobną ścieżką:

  • Autozdejmowanie mute'a po dwóch godzinach? Spróbowałem zaimplementować za pomocą System.Timer ale... rzuca wyjątkami! Pewnie by się dało zaimplementować podobnie, jak timery do gry (tj. gdyby instancja klasy System.Timer była w jakimś Singletonie), no ale już nie chcę Singletonów.
  • Banowanie adresów IP. Bo przecież zmute'owanie konta nie ma sensu, jeśli gość może sobie zmienić nick (np. z Jerkass1 na Jerkass2) i dalej spamować czat.

Ja wiem - to pewnie wynika z mojej niedostatecznej znajomości frameworka. Ale zdumiewa mnie, ile czasu jest porzebne, by rozwiązać zdawałoby się najprostsze problemy.

Szalony Programista

Czasem idzie skrócić te ify i wtedy człowiek jest zadowolony, że się do czegoś ta matematyka przydała.

WeiXiao

@Szalony Programista: ale nikt nie powiedział, że wyszłoby ich dużo :P

kmph
2018-09-04 19:15

Tym razem mniej techniczny wpis w temacie #mon .

Korzystam ze studenckiego przywileju i we wrześniu siedzę jeszcze na wsi, na działce rodziny. Tam też próbuję dalej dziergać projekt. Pogoda dopisuje, w sam raz na codzienne rowerowe wyjazdy do sklepu do pobliskiego miasteczka.

Jest natomiast krucho z Internetem. Skazany jestem na co chwila tracącego zasięg palucha. Z jednej strony to dobrze, bo League of Legends i takie tam zjadają mniej czasu... ale z drugiej strony, sprawdzenie dokumentacji (MDN czy docs.microsoft.com) to męka.

A już miałem napisać, że ostatnio jest wyraźna poprawa. Już miałem napisać, że pewnie wzmocnili jakiś maszt albo postawili nowy. Bo rzeczywiście, przez jakiś czas internet, choć chodził powoli, to jednak był w miarę stabilny.

Ale od paru dni mam znów powtórkę tego, co pamiętam od lat na tej działce: internet co chwila się rozłącza. Nierzadko zanim potrzebna strona zdąży się załadować. I zamiast dokumentacji widzę stronę błędu przeglądarki. Po 5 min znów na chwilę złapie zasięg, więc próbuję znów, i znów z takim samym rezultatem.

Nie wiem, od czego to zależy? Od pogody?

W ogóle zresztą ten modem 3G jest jakoś zrypany. Jeśli mi na numer przyjdzie SMS, to paluch... odcina mi internet. Teoretycznie ma to działać tak, że przekierowuje mi ruch na własny lokalny adres IP, na swój panel administracyjny, gdzie wyświetla mi, że przyszedł mi SMS i że jeśli chcę kontynuować przeglądanie internetu, mam uruchomić ponownie przeglądarkę. W praktyce wygląda to tak, że modem niektóre strony mi puszcza, inne blokuje mi dokumentnie (błąd przeglądarki się pojawia) a dopiero niektóre rzeczywiście przekierowuje na swój adres. I teraz jak nagle nie mam internetu, to się muszę zastanawiać, czy znów straciłem zasięg, czy też SMSa otrzymałem, a jeśli tak, to na którą stronę muszę spróbować wejść, żeby przekierowanie wreszcie zadziałało i żeby mi się odblokował internet. Na szczęście zapamiętałem, że wejście na stronę kina Luna zawsze wywołuje to przekierowanie.

Stwarza to jeszcze inne problemy. Oto np. chciałem wejść na panel administracyjny konta w Plusie. Już nie pamiętam, włączyć / wyłączyć jakieś usługi chyba. Czy tam rachunek zobaczyć. Ale Plus wymaga 2FA. No więc dochodzi mi SMS... I paluch blokuje mi internet. Jak odczytuję SMSa z kodem weryfikacyjnym, muszę uruchomić ponownie przeglądarkę. A wtedy muszę przechodzić 2FA na nowo! Na szczęście ten wymóg ponownego uruchamiania przeglądarki paluch też losowo jakoś implementuje. Czasem rzeczywiście, nie mogę przeglądać innych stron póki ponownie Firefoxa nie uruchomię, kiedy indziej internet działa i bez tego. (Skąd modem 3G może wiedzieć, czy ponownie uruchomiłem Firefoxa, czy nie??)

Też ciekawe - chociaż to modem 3G, to jednak rejestruje się w systemie jako połączenie Ethernet, a nie jako połączenie komórkowe.

W Plusie kupione dziadostwo!!

Jeż mi się na działce zadomowił. Niestety widać go przede wszystkim wieczorami (nocne stworzenie?) Trudno odróżnić go od kępki trawy po ciemku. Raz go niechcący mocno pokopałem z tego względu. To mnie nauczyło, by patrzeć pod nogi. Za drugim razem, gdy tylko mnie zwęszył, od razu zwinął się w kolczatą kulkę i piszczał.

Aryman1983

ależ się rozgadał na tym microblogu :-) Nie wiem czy to działa, ale jak chcesz to zostaw sobie na noc https://social.msdn.microsoft[...]ity?forum=visualstudiogeneral i prawdopodobnie będziesz miał całą dokumentację jaką potrzebujesz :-)

kmph
2018-09-03 15:45

Follow-up do ostatniego postu w temacie przechowywania rozgrywek #mon w słowniku w pamięci.

Dobrze - przekonaliście mnie - zamierzam pozbyć się słownika w pamięci, wszystkie rozgrywki - tak trwające, jak i minione - przechowywać w bazie danych. Niemniej, jak wiele innych poprawek, roboczo odłożyłem sobie to zadanie na później, tj. do czasu, gdy skończę prototypować zaplanowane funkcjonalności.

No a jedną z tych funkcjonalności była obsługa replayów oraz pozwolenie na obserwowanie rozgrywki, w której nie bierze się udziału. W tym celu, myślę sobie, stan gry powinien być blokowany w jakiś mądrzejszy sposób, aniżeli lock - wydawało mi się, że to jest typowy przypadek czytelników i pisarzy, więc postarałem się użyć ReaderWriterLockSlim. Myślałem, że implementacja tego będzie szybsza, niż od razu pozbycie się słownika i oparcie się tylko o bazę danych. Chyba jednak się myliłem.

Oto (w dużym skrócie) kod BattleState i BattleManager, jaki mi powstał. Kod ten jest niepoprawny, o czym dowiedziałem się już po napisaniu go :(

public class BattleManager : IBattleManager
{
    private ulong next_id = 0;
    private readonly object next_id_lock = new object();
 
    public ConcurrentDictionary<ulong, BattleState> battles = new ConcurrentDictionary<ulong, BattleState>();
 
    public BattleMetaInfo StartBattle(Dictionary<UserInfo, Team> players)
    {
        ulong id;
        lock(next_id_lock)
            curr_id = next_id++;
        var battleState = new BattleState(players, curr_id, this);
        var ret = new BattleMetaInfo(battleState);
        battles[curr_id] = battleState;
        return ret;
    }
 
    public BattleTurnInfo SendCommand(ulong battleId, UserInfo sender, Move command)
    {
        BattleState battleState;
        try {
            battleState = battles[battleId];
            return battleState.update(sender, command);
        } catch(KeyNotFoundException) { return null; }
    }
 
    // Ktoś chce obserwować rozgrywkę, trzeba mu wysłać wszystko, co stało się aż dotąd.
    public BattleTurnInfo GetProgress(ulong battleId)
    {
        BattleState battleState;
        try {
            battleState = battles[battleId];
            return battleState.getUpdates();
        } catch(KeyNotFoundException) { return null; }
    }
 
    // Ktoś chce zobaczyć, jakie rozgrywki obecnie trwają
    public List<BattleMetaInfo> GetOngoingBattles() =>
        battles.Values.Select(b => b.getMetaInfo()).Where(metaInfo => metaInfo != null).ToList();
 
    // Ktoś kliknął na nick innego gracza
    public List<BattleMetaInfo> GetPlayersBattles(string nick) =>
        GetOngoingBattles().Where(metaInfo => metaInfo.players.Contains(nick)).ToList();
 
    // Ktoś wpisał w searchboxa ID rozgrywski
    public BattleMetaInfo GetBattleById(ulong id)
    {
        BattleState battleState;
        battles.TryGetValue(id, out battleState);
        return battleState?.getMetaInfo();
    }
}
 
public class BattleState
{
    private ReaderWriterLockSlim rwlock = new ReaderWriterLockSlim();
    private bool isBeingRemoved = false;
    private readonly BattleManager battleManager;
    private Timer timer = new Timer() { Interval = 120000, AutoReset = false };
 
    public BattleState(Dictionary<UserInfo, Team _players, ulong _battleId, BattleManager _battleManager)
    {
        timer.Elapsed += (sender, e) => timerRunOut(sender, e);
        timer.Start();
        // .. i więcej kodu tu nieistotnego
    }
 
    public void timerRunOut(object sender, ElapsedEventArgs e)
    {
        try
        {
            rwlock.EnterWriteLock();
            // Zakańczamy grę i ogłaszamy przegranym tego, czyja jest teraz kolejka
        }
        catch (ObjectDisposedException) { }
        finally
        {
            try { rwlock.ExitWriteLock(); } catch { }
            _ = removeBattleAsync();
        }
    }
 
    public BattleTurnInfo update(UserInfo sender, Move command)
    {
        bool mustBeRemoved = false;
        try
        {
            rwlock.EnterWriteLock();
            // Dokonujemy odpowiednich zmian i jeśli ten ruch zakańcza rozgrywkę, to mustBeRemoved jest true
        } catch (ObjectDisposedException) { return null; }
        finally
        {
            try { rwlock.ExitWriteLock(); } catch { }
            if (mustBeRemoved) _ = removeBattleAsync();
        }
    }
 
    public BattleTurnInfo getProgress()
    {
        try
        {
            rwlock.EnterReadLock();
            return new BattleTurnInfo(this);
        } catch( ObjectDisposedException) { return null; }
        finally
        {
            try { rwlock.ExitReadLock(); } catch { }
        }
    }
 
    public BattleMetaInfo getMetaInfo()
    {
        try
        {
            rwlock.EnterReadLock();
            return new BattleMetaInfo(this);
        } catch( ObjectDisposedException) { return null; }
        finally
        {
            try { rwlock.ExitReadLock(); } catch { }
        }
    }
 
    public async Task removeBattleAsync()
    {
        lock(this)
            if (isBeingRemoved) return;
            else isBeingRemoved = true;
 
        try
        {
            using (var scope = battleManager.scopeFactory.CreateScope())
            {
                var db = scope.ServiceProvider.GetRequiredService<ApplicationDbContext>();
                var replay = new Replay(battleId, this, DateTime.Now.ToUniversalTime());
                await db.replays.AddAsync(replay);
                await db.SaveChangesAsync();
            }
        }
        finally
        {
            ((IDictionary<ulong, BattleState>)battleManager.battles).Remove(battleId);
            timer.Dispose();
            rwlock.Dispose();
        }
    }
}

Te wszystkie catch (ObjectDisposedException) są po to, żeby chronić się przed sytuacją np. taką: wątek 1 to żądanie przesłania historii rozgrywki (ktoś wołał GetProgress), ale wątek 1 czeka, bo w międzyczasie wątek 2 ma WriteLock i właśnie zakańcza rozgrywkę. Nie jest deterministyczne, kiedy wątek 1 się dostanie - może się najpierw odpalić Dispose. Ale wtedy po prostu zwracamy null i caller (hub SignalR) wie, że ma zamiast tego szukać już w bazie danych. (Bardziej prawdopodobny przykład: żądanie GetProgress przychodzi tuż przed usunięciem bitwy ze słownika. Przeplot: Wątek 1 wyciąga bitwę ze słownika i posiada referencję do niej, wątek 2 odpala ((IDictionary<ulong, BattleState>)battleManager.battles).Remove(battleId); oraz rwlock.Dispose, wątek 1 próbuje założyć ReadLock).

Ale to jest też powód, dla którego powyższy kod jest niepoprawny. Już po napisaniu go przeczytałem, że według oficjalnej dokumentacji ReaderWriterLockSlim.Dispose() rzuca wyjątkami, jeśli są jakiekolwiek wątki, czekające na otrzymanie locka. Jak ja mam zapewnić, że rwlock.Dispose() wykona się dość późno? Tak późno, że żaden wątek już nie czeka ani czekać nie będzie na blokadę? Sytuacja wydaje mi się beznadziejna i dlatego chyba należy ten kod wyrzucić do kosza. Nie mogę tu użyć ReaderWriterLockSlim.

@Afish napisał:

Odnośnie blokad i Dispose - nie widziałem kodu, ale Twój opis sugeruje, że robisz coś bardzo źle.

OK - co tu robię bardzo źle? Pytanie w zasadzie teoretyczne, chcę po prostu się czegoś nauczyć, bo jak napisałem wyżej, nie widzę możliwości wykorzystania tego kodu.

Chyba, że... Jak bardzo złe byłoby rozwiązanie, w którym wołam ReaderWriterLockSlim.Dispose() w destruktorze BattleState? Na Stacku czytam, że Garbage Collector się tu nie myli i nie zawoła tego za wcześnie. Czy to rozwiązanie jest z gatunku tych złych, czy tych tragicznych? Wiem, że to będzie koniecznie wymagało zmiany prędzej czy później, ale może umożliwi mi to doprowadzenie jak najszybciej kodu do działania, bym mógł w międzyczasie zająć się prototypowaniem kolejnych funkcjonalności...

cs

Czegoś nie rozumiem, jeśli EnterWriteLock w bloku try rzuci wyjątkiem, co może znaczyć, że nie mógł zająć blokady, to obowiązkowe zwolnienie w bloku finally jest bez sensu, tzn. nie wiem czy można bezkarnie zdejmować nieistniejącą blokadę w lock'u. Po drugie czy EnterWriteLock może w ogóle rzucić wyjątkiem? Po trzecie nie rozumiem "bo po wzięciu locka wątek może polecieć wyjątek i finally się nie wykona", jeśli finally się nie wykona to chyba najmniejszym problemem jest wtedy deadlock, Po czwarte zamiast twardo zajmować WriteLock można spróbować przez TryEnterWriteLock z timeoutem.

Afish
  1. Zazwyczaj nie można bezkarnie zdejmować blokady, ale można zapytać locka, czy udało się wziąć blokadę.
  2. Tak.
  3. Jeżeli masz sensownie napisaną aplikację, to ten wyjątek jest złapany gdzieś wyżej i zalogowany, aplikacja działa dalej, ale jest problem, bo blokada jest wzięta i nic już jej nie zwolni.
  4. A to swoją drogą, że nigdy nie powinno się brać blokady bez timeouta, ale to nie ma znaczenia w tej dyskusji.
kmph
2018-08-28 01:54

Ostatnio opisywałem pierdółkę. Teraz jednak mam problem IMO poważniejszy. Chociaż mam działające rozwiązanie, jednak na >90% jest to rozwiązanie dalece nieoptymalne. Nie wiem, jakie rozwiązanie jest prawidłowe. Kwestia związana jest mocno z architekturą całego projetku, tym samym post będzie dłuższy.

Problem wydaje się być związany z moją niedostateczną znajomością tego - jak to się nazywa? - "stacka techonologicznego"? - którego używam, tj. C# + .NET Core (w tym Entity Framework, SignalR itd).

Gra jest przez sieć. Pojedyncza rozgrywka to jest 1 na 1 gracz. Gra jest turowa, przy czym tury niekoniecznie muszą być na przemian (czasem ktoś ma 5 tur pod rząd, czasem obaj mają turę jednocześnie - wtedy tura zaczyna się dopiero, gdy obaj wydadzą polecenia.)

Architektura gierki, z jaką zaczynałem, jest następująca: Mam sobie klasę BattleState, która zawiera trwającą rozgrywkę. Mam też klasę BattleManager, która zarejestrowana jest jako Singleton Service. BattleManager zawiera ConcurrentDictionary, który mapuje long Id do BattleState. Gdy ktoś w przeglądarce wyklikuje sobie, jak ma wyglądać jego tura, to te polecenia odbierane są (wraz z Id bitwy) przez huba SignalR, który zależy (na mocy wymuszonego przez framework dependency injection) od BattleManagera. Tenże BattleManager ma metodę public BattleTurnInfo SendCommand(ulong battleId, UserInfo player, Move command), którą wywołuje hub. Wówczas BattleState o tym Id jest wyciągany z ConcurrentDictionary, blokowany, i o ile jest to możliwe (np. nie czekamy wciąż na ruch przeciwnika), to popychamy grę do przodu o 1 turę i zwracamy hubowi postępy w rozgrywce za pomocą BattleTurnInfo (jednocześnie usuwając grę ze słownika, o ile to jest już koniec gry i mamy zwycięzcę). Hub zaś owe postępy rozgłasza po obu graczach (temu, który się ruszył, i jego przeciwnikowi).

Przypuszczam, że to już idzie w poprzek zastosowaniom, do jakich przewidziany jest framework. Przypuszczam, że teoretycznie powinno to wyglądać raczej tak: Gdy tylko ktoś się rusza, to z bazy danych wyciągany jest (i blokowany!) wiersz o Id danej rozgrywki; stan gry jest deserializowany do obiektu BattleState, gra jest popychana naprzód o ile to możliwe, hub rozgłasza postępy o ile ma co rozgłaszać, tymczasem BattleState jest ponownie serializowany i zapisywany do bazy danych. Nie ma żadnych Singletonów, nie ma żadnych ConcurrentDictionary'ów, które udają bazę danych: jest tylko baza danych i wszystkie niezbędne Services są albo Scoped, albo Transient.

Czemu więc nie zrobiłem tego w ten sposób? Z dwóch powodów, jeden był ideologiczny, a drugi praktyczny:

  1. Ideologiczny: Takie wykorzystanie bazy danych wydaje mi się być dziwne. Wydaje mi się to być używanie bazy danych tak, jakby to był RAM. Baza danych to dla mnie jest mechanizm persystencji danych, tj. używamy jej raczej w zastępstwie dysku twardego, nie RAMu: czyli nie zapisujemy i nie wyciągamy z bazy w kółko tych samych wierszy, tylko zapisujemy coś, gdy skończyliśmy na tym operować i odkładamy to na później, by nie zaśmiecały RAMu i by przetrwały ubicie i ponowne uruchomienie aplikacji. Zatem - jak ja to widzę - zapisywanie do bazy trwających rozgrywek nie ma sensu, bo wedle wszelkiego prawdopodobieństwa za chwilę będziemy musieli wyciągać je z powrotem. (Przypuszczalnie nie tak się sprawy mają?) (Na marginesie - gdyby gra była realtime, to już naprawdę nie widzę, jakie inne rozwiązanie niż moje powinno być zastosowane - no może oprócz użycia Unity3D).
  2. Praktyczny: Entity Framework NIE MA SelectForUpdate, tak jak ma Django - w takim razie, nawet pomijając opisaną wyżej ideologię, pojęcia nie mam, jak to zaimplementować. Naprawdę wydaje mi się, że MUSZĘ zablokować rozgrywkę i czekać na jej odblokowanie

OK, działa. Aż do czasu, gdy przychodzi pora na zaimplementowanie timera. Uznałem to za niezbędne, bo inaczej niektóre bitwy mogłyby w ogóle nigdy nie opuścić dicta (tj kiedy któryś gracz w trakcie bitwy się rozłącza i nigdy nie wraca). Timera wyobrażałem sobie tak: Jeśli zaczyna się teraz kolejka gracza A, to zaczynamy liczyć mu 2 minuty na zrobienie tej kolejki. Jeśli się nie zmieści, to natychmiast zakańczamy grę i uznajemy go za przegranego.

Tutaj, z racji braku wiedzy, w ogóle nie wiedziałem, jak się za to zabrać. Uciekłem się do zadania pytania na Stacku. Tam zaproponowano mi na początku zrobienie jakiegoś "housekeeping task" do tego celu, później jednak stwierdzili, że do takich timerów jest to jednak nienajlepszy pomysł. Zamiast tego zaproponowano mi użycie klasy System.Timers.Timer.

Tak też uczyniłem. BattleState zawiera sobie pole typu System.Timers.Timer i kiedy ktoś się ruszy, to dodatkowo: (a) Zatrzymujemy jego timer; (b) Po popchnięciu gry naprzód nastawiamy timer na kolejne 2 min. (I fakt ten rozgłaszamy po graczach w ramach BattleTurnInfo zwracanego przez SendComman. No ale dobrze, teraz ktoś za długo czekał, Timer się aktywuje. Więc timer woła odpowiednią metodę na BattleState, która blokuje się, zakańcza rozgrywkę i woła BattleManager, by usunął ją z dicta.... no i uff, mamy problem - to bo teraz trzeba jakoś rozgłosić koniec gry. Przynajmniej tego drugiego poinformować, że wygrał.

Szukanie po Stacku doprowadziło w końcu do rozwiązania, które działa, ale obawiam się, że i tak jest błędne. Mianowicie okazało się, że jeśli wsadzę tego huba SignalR do konstruktora BattleManager, to dependency injection mi go dostarczy. Wtedy mogę sobie rozgłosić z tego huba co chcę, mimo że entry point to nie jest ten hub, tylko BattleManager.

Jednak! W ten sposób instancja tego huba trwa przez cały czas życia aplikacji na serwerze! Czy tak wolno? Co gorsza: Może się zdarzyć, ze dwie gry w tym samym czasie będą miały zakańczające się timery. Czy instancja hubu SignalR jest thread-safe? Bo jeśli nie, to muszę blokować go? Ale jeśli go zablokuję, to tym samym zaprzeczę sensowi używania ConcurrentDictionary i klasy BattleManager, która jest - ma być - thread safe. PRzecież ten framework ma być zaprojektowany, by działał na wielu wątkach.

Niedawno jednak pojawił się kolejny problem, na który znów nie miałem odpowiedzi. No bo chcę wreszcie zaimplementować to zapisywanie zakończonych gier do bazy danych, by móc obsługiwać replay'e. Zatem muszę mieć dostęp do Entity Framework Core. No ale Entity Framework jest Scoped: zakazane jest wywoływanie Scoped w Singletonach. Mogę co prawda uczynić hub SignalR zależnym od EF, wtedy dependency injection da mi EF(skąd, drogą eliminacji, musimy wnieść, że hub SignalR jest Transient?)... no ale znowu: z powodu timerów, MUSZĘ BYĆ W STANIE WYWOŁAĆ BAZĘ DANYCH Z BattleManagera! Czyli zrobić dokładnie to, czego framework mi zakazuje.

Kolejne pytanie na Stacku. Okazuje się że rzeczywiście, można jednak (technicznie) w Singletonie zażądać sobie jakiegoś Scoped Service. W związku z tym metoda odpowiadająca za zakańczanie rozgrywek w BattleManager wygląda teraz następująco:

        // The battle here must be locked by the caller
        public void removeBattle(ulong battleId)
        {
            // Only this place of code removes battles. So I think there may be no such leaks that some timers are left undisposed?
            var battleState = battles[battleId];
            battleState.disposeTimers();
            ((IDictionary<ulong, BattleState>)battles).Remove(battleId);
 
            var replay = new Replay(battleId, battleState, DateTime.Now.ToUniversalTime());
            var playerIds = battleState.players.Keys.Select(userInfo => userInfo.UserIdentifier);
            _ = saveReplayAsync(replay, playerIds);
        }
 
        public async Task saveReplayAsync(Replay replay, IEnumerable<string> playerIds)
        {
            using (var scope = scopeFactory.CreateScope())
            {
                var db = scope.ServiceProvider.GetRequiredService<ApplicationDbContext>();
                await db.replays.AddAsync(replay);
                foreach (var playerId in playerIds) await db.AddAsync(new ReplayPlayers(replay, playerId));
                await db.SaveChangesAsync();
            }
        }

Tak - metoda removeBattle zakłada, że odnośny BattleState jest zablokowany przez callera. Może to i jest spaghetti, ale naprawdę nie mam pomysłu na nic lepszego.

Powyższy kod wołany jest zarówno, gdy gra się skończy normalnie (któryś z graczy wykonuje wygrywający ruch), jak i wówczas, gdy timer się zakańcza.

Działa. Ale teraz to już jestem pewien, że odstawiam cuda na kiju i że framework absolutnie nie jest przewidziany do podobnych zabaw. Co gorsza, w powyższym pytaniu na Stacku dowiedziałem się jednak, że nie mogę mieć wielu Timerów aktywnych jednocześnie, choćby z powodu marnowania pamięci i procesora (i to, podobno, sporego marnowania)! Zamiast tego mam jednak zrobić "background task", odpalający się co np. 20 sek! Nie rozumiem tego. Przecież wtedy Timery będą się zakańczać z dokładnością do 20 sek. To znaczy, że gdy graczowi skończy się czas, to jego przeciwnik będzie musiał czekać do 20 sek., aby oficjalnie zostać zwycięzcą? No nie powinno tak to wyglądać.

Aha. Ludzie na Stacku sobie przeczą. No bo, o ile dobrze rozumiem, pod pierwszym pytaniem odradzano mi dokładnie to samo rozwiązanie, które pod drugim pytaniem mi doradzano? No cóż, można się było tego spodziewać. Ale w takim razie co ja mam zrobić, skoro mnie samemu brak wiedzy, by rozeznać?

Hmm... planuję kolejne posty kwestii tej gry.. aż przydałby sie tag... #mon

kmph

@Afish: A nie muszę robić czegoś na tej zasadzie? https://docs.microsoft.com/en-us/ef/core/saving/concurrency Tak jak rozumiem transakcje, to one gwarantują mi TYLKO, że albo wszystkie zmiany są zapisane, albo żadna. Ale przecież nie do końca przed tym chcę się bronić. Załóżmy np., że mamy turę, w której wymagane są komendy od obu graczy, by móc ruszyć dalej grę. Tak więc BattleState ma zaznaczone, iż oczekuje na komendy od obu. Teraz - co chyba wcale nie jest nierealne? - obaj niemal naraz wysyłają swoje komendy. Więc teraz tak: wątek 1 otwiera transakcję z komedną od gracza 1, wątek 2 otwiera transakcję z komendą od gracza 2, wątek 1 zapisuje komendę gracza 1, zaznaczjąc, że teraz czeka się na komendę od gracza 2 i zamyka transakcję, jako że wątek 2 otworzył transakcję, to operuje on wciąż na danych sprzed zapisu wątku 1, więc zapisuje komendę gracza 2, zaznaczając, że teraz czeka się na komendę od gracza 1 i... zamyka transakcję, nadpisując komendę od gracza 1, która jest stracona! Tak chyba jednak być nie może?

AD. EF będącym scoped: Sprawdzałem: Próba wsadzenia database context do konstruktora SingletonService rzuca wyjątkami. Oczywiście na upartego z pewnością da się to obejść i wsadzić jednak kontekst singletonowi, jednak dokumentacja przestrzega przed tym bardzo: https://docs.microsoft.com/en[...]injection?view=aspnetcore-2.1 Strasząc invalid states i takimi tam.

Jednak, moze jeśli mamy pozbyć się tego ConcurrentDictionary i oprzeć się tylko na bazie danych, to nie ma to znaczenia, bo już nie trzeba mieć żadnych Singletonów? A tak zamierzam zrobić, i to już, nie później. Gdyż poległem na blokadach. Chcę zaimplementować obserwowanie trwających rozgrywek oraz listowanie ich. W tym celu chciałem blokować BattleState za pomocą ReaderWriterLockSlim. I co? Gucio... gdyż z jednej strony muszę je Dispose'ować, a z drugiej strony, nie wolno mi tego zrobić, póki jakikolwiek wątek czeka na blokadę! Jak mam to zapewnić? O tym dowiedziałem się dopiero poniewczasie, muszę wyrzucić nieco kodu do śmieci.

Afish

Odnośnie transakcji - nie rozumiesz poziomów izolacji i literki I z ACID-a.
Odnośnie EF - nie znam DI z core'a, więc nie pomogę.
Wszystko, co chcesz zrobić na bazie, możesz równie dobrze zrobić na słowniku w pamięci, tylko musisz więcej kodu napisać.
Odnośnie blokad i Dispose - nie widziałem kodu, ale Twój opis sugeruje, że robisz coś bardzo źle.

kmph
2018-08-25 16:33

I dalszy ciąg problemików z moją gierką...

Tym razem problem krótki. Konwencje nazewnictwa.

Jakoś projekt nazwać musiałem, wymyśliłem więc roboczą nazwę mon. Zielonego pojęcia nie miałem, że Visual Studio stworzy mi namespace mon i jeszcze wetknie tam całkiem sporo kodu z szablonu.

No i teraz problem... No bo zorientowałem się, że konwencja jest, że namespace'y pisane są Wielkimi Literami. A więc: powinno być Mon, a nie mon. No więc, swój kod zacząłem wprowadzać w namespace Mon.

Jednak jest niekonsekwencja. Bo po pierwsze, jak już pisałem, Visual Studio już mi wsadził sporo kodu w namespace mon. Mało tego: sam jestem niekonsekwentny... bo jak tylko klikam, że chcę "new class", to automatycznie szablon wtyka mi ją w namespace mon. Jak zauważę, to poprawię na Mon, ale robi się to żmudne, a poza tym raz na jakiś czas coś przeoczę i parę fragmentów kodu wisi mi w mon. Ah, teraz zauważyłem... migracje itp. też są automatycznie umieszczane w mon!

Przede mną zatem przejrzenie i poprawienie zescaffoldowanego kodu... no i dokopanie się do settingu, żeby szablon przestał mi wiecznie umieszczać nowe pliki w namespace mon.

I kolejne problemy z konwencjami. No bo konwencja jest, że metody, properties i publiczne pola też są Wielkimi Literami. Ale ja jestem przyzwyczajony, że się je pisze małymi... Odruchowo piszę klasy i namespace'y Wielkimi, a wszystko inne małymi. W zasadzie wypadałoby to wszystko pozmieniać. No ale to żmudna robota, nie chce mi się, a poza tym nie mogę oprzeć się wrażeniu, że są ważniejsze rzeczy do poprawienia, aniżeli ta pierdópołka. Więc to za te ważniejsze się biorę.

Ale w takim razie ta dziwna konwencja pozostaje. No bo skoro już taka konwencja mi się zrobiła, no to dalej piszę według niej. Tym więcej zostawiam sobie do poprawiania na później.

I wreszcie bodaj najdziwniejszy problem:

public class SomeClass
{
    public SomeClass(int _someField, string _otherField)
    {
        someField = _someField;
        otherField = _otherField;
    }
 
    public int someField;
    public string otherField;
}

Tak wiem, te public biją po oczach, wiem że to wada, ale o tym już wcześniej pisałem. Teraz chodzi mi o co innego: o tę dziwną konwencję, że parametry konstruktora zaczynają się od _. Wzięło się to z tego, że nie wiedziałem, czy i jak w ciele konstruktora mogę rozróżnić między polem a parametrem o tej samej nazwie. Od początku przypuszczałem, że jest szansa, że zadziała coś takiego:

public class SomeClass
{
    public SomeClass(int someField, string otherField)
    {
        this.someField = someField;
        this.otherField = otherField;
    }
 
    public int someField;
    public string otherField;
}

Ale nie byłem pewny i nie sprawdziłem... gdyż byłem skoncentrowany na innym problemie... i już nie chciałem rozmieniać się na drobne. Więc zdecydowałem się na ten kludge.

Tak naprawdę dotąd nie wiem, czy powyższy kod (z tymi this) działa. Wiem natomiast, że jeśli już, to konwencja jest przeciwna: To pola (które nie maja być publiczne) mogą mieć _ przed nazwą, ale nie parametry konstruktora.

No i znowu - konwencja została, nie zmieniam wszystkiego no bo żmudne to (dowiedzenie się, jak powinno być, jest najłatwiejsze tutaj)... no i piszę dalej tak samo, no bo trzymam się tej już utartej konwencji.

Następny post będzie dłuższy... bo i problemik w nim opisany (dla mnie) trudniejszy.

#mon

furious programming

Zmienić go – nigdy nie jest za późno na refactoring. Obstawiam, że IDE którego używasz, posiada funkcjonalność wygodnej zmiany identyfikatorów (co aktualizuje wszystkie ich wystąpienia we wszystkich modułach), dzięki czemu nie będziesz musiał tego robić ręcznie.

Sam niedawno pozbyłem się prefiksów L dla zmiennych lokalnych w moim platformersie, który obecnie składa się z sześciu projektów, kilkudziesięciu modułów, łącznie z około czternastu tysięcy linii kodu – nie trwało to dłużej niż kwadrans. Gdybym miał ręcznie wszystko zmieniać, ciąg po ciągu, to siedziałbym przy tym dwa dni.

somekind

W ustawieniach projektu w Visual Studio można zmienić global namespace na prawidłową, wtedy nowo tworzone typy będą się w niej znajdowały. Co do starych namespaców, to albo ctrl + shif + f albo ReSharper -> Adjust namespaces, albo jakaś wtycznka, np.: https://marketplace.visualstu[...]itemName=p2410.NamespaceFixer

kmph
2018-08-13 20:04

Walki z projektem ciąg dalszy. Chyba nie zdążę przed końcem wakacji zrobić wszystkiego, co planuję :(

Mam dość poważne problemy z duplikacją kodu. Niestety, choć staram się to ograniczać (o ile tylko owo ograniczanie nie zatrzyma mnie zbyt mocno), copypasty jest tam nadal zdecydowanie za dużo. Nie wszędzie mam rozsądne pomysły, jak się tego pozbyć.

Weźmy na przykład public class Block : IAction. Mój oryginalny pomysł był taki, że utrzymywanie bloka (a) kosztuje niewielką ilość Staminy na turę, jeśli Staminy nie ma to blok jest zdejmowany; (b) natychmiast, gdy przeciwnik ukończy swój ruch, blok jest zdejmowany. Interfejs IAction wymaga metody execute - świetnie zatem, niech Block implementuje execute w ten sposób, że zabiera nieco Staminy, sprawdza, czy przeciwnik jest wolny i jeśli tak, zdejmuje się.

Działa. Ale! Oprócz Block mam jeszcze i Dodge, które działa bardzo podobnie, tyle że zamiast blokowania ataku przeciwnika (pewne zmniejszenie obrażeń o % wynikły ze stosunku Power walczących) usiłuje wykonać unik (przyjęcie albo 0% albo 100% obrażeń w zależności od RNG, szansa zależy od stosunku Speed obu walczących). No i obie klasy duplikują kod odpowiedzialny za zdejmowanie się wskutek braku Staminy albo wskutek faktu, że przeciwnik zakończył już swój ruch.

Co gorsza: Za chwilę okazuje się, że jednak Block oraz sposób jego nakładania i zdejmowania to są dwie różne rzeczy. No bo teraz np. chcę zrobić taki ruch jak Ram. Działa on tak: user podbiega do przeciwnika, blokując podczas podbiegania, następnie atakuje i zdejmuje bloka. No i mam problem. Jak to zrobić bez duplikacji kodu? Duplikowanie całej klasy Block oprócz kodu odpowiedzialnego za zdejmowanie to już przesada, nawet dla mnie.

Chcę wielodziedziczenia! Wtedy, myślę, dałbym sobie klasę Block, klasę Dodge, klasę RequireStaminaPerTurn, klasę UntilEnemyFinishesMove, klasę UntilEndOfChannel. I teraz niech NormalBlock dziedziczy po Block, RequreStaminaPerTurn, UntilEnemyFinishesMove. I niech NormalDodge dziedziczy po Dodge, RequireStaminaPerTurn, UntilEnemyFinishesMove. I wreszcie niech ChannelBlock dziedziczy po Block, RequireStaminaPerTurn, UntilEndOfChanel. Wszystko. Teraz nawet można (teoretycznie, nie zrobiłem tego) dać postać, która będzie miała taki passive, że będzie mieć bez przerwy założonego bloka bez żadnych kosztów Staminy.

Niestety! Wielodziedziczenie jest be i nie ma go w C#. Coś jednak muszę wymyślić! Z jednej strony nie chcę duplikować kodu ponad miarę, a z drugiej strony, nauczony tym doświadczeniem, chcę wprowadzić i utrzymać zasadę, że żaden efekt mający wpływ na grę nie odpowiada za swoje własne zdejmowanie i nakładanie.

W końcu wymyśliłem. Block nie implementuje już IAction. Zamiast tego pojawiły się takie klasy, jak: public class PlaceEffect: IAction, public class RemoveEffect : IAction, taki interfejs: public interface RequireSetup : IEffect { List<IAction> onPlacement { get; } List<IAction> onRemoval { get; } List<IAction> onFailure { get; } } itepe, itede.

Biorąc pod uwagę fakt, że dodatkowo jeszcze, celem umożliwienia, by tury wykonywały się jednocześnie (w sensie, obaj gracze dają polecenia i wtedy wykonuje się tura, nie ma znaczenia który gracz wykonuje swą turę jako pierwszy) wprowadziłem wymóg, by każda IAction implementowała property o nazwie priority - no zrobiło mi się spaghetti. Te wszystkie priority, getPriority, PlaceEffect, onBlahblah => { new List<IAction> { new PlaceEffect(_priority: yaddayadda, _effect: new JakiśtamEfekt(_onFailure : new List<IAction> {new RemoveEffect(plepleple.... nie chce mi się kończyć otwartych nawiasów i tym podobne... Wydaje mi się że brzydkie, obawiam się że błędogenne, i co gorsza podobne ruchy powtarzają ten niemiły kod czyli i tak mamy copypasty.

No ale idziemy dalej. Teraz implementuję sobie damage over time. Na pierwszy ogień idzie Hurricane i Conflagration. Te efekty odnoszą się do całego Field, toteż atakują obu graczy. No i tu już mamy problem, gdyż to ma działać jako efekt "stackowalny", tj: wykonanie ruchu Hurricane nakłada stack efektu Hurricane który to stack zostanie zdjęty po czasie zależnym od statystyk gracza nakładającego ten stack. Tak samo Conflagration. Co więcej, Hurricane i Conflagration razem tworzą kombo FieryTornado. I wreszcie, Hurricane, oprócz damage over time, nakłada jeszcze na obu graczy taki debuff, że każdy ich ruch kosztuje nieco więcej Staminy.

Zaimplementowane. Mam klasę Weather : IExecutable<Field>, IOnTire. Implementowanie IExecutable<Field> jest po to, by mogła nakładać obrażenia każdej tury. Implementowanie IOnTire jest po to, by mogła zwiększać zużycie Staminy. W niej klasy: PlaceWeatherStack : PlaceEffect, RemoveWeatherStack : RemoveEffect (ich zadanie: nakładanie / zdejmowanie instancji klasy Weather do zbioru efektów na Field kiedy tylko zostanie nałożony jakiś stack Hurricane lub Conflagration / kiedy ostatni stack zostanie zdjęty); no i oczywiście klasy HurricaneStack, ConflagrationStack. Klasa Weather, wywołana, sumuje sobie wszystkie stacki i na tej podstawie zadaje obrażenia / zwiększa użycie Staminy. Powinno działać. (Jeszcze nie przetestowałem).

Niestety. Klasa Weather, ze wszystkimi klasami zagnieżdżonymi, to jest sporo kodu. Więcej, niżbym przewidywał. 200 linii z hakiem.

A teraz nadchodzą kolejne dwa efekty: ToxicSpores i BurningSpores. Także zadają damage over time, ale już tylko na przeciwników. Nie mają także dodatkowych efektów takich jak IOnTire. Mimo to... duplikacja kodu pomiędzy tymi klasami a Weather jest straszna. Nieakceptowalna nawet dla mnie.

Ale przecież nie chcę! zrobić klasy bazowej, choćby abstrakcyjnej, która by zawierała w sobie zduplikowany kod! Przecież nie ma wielodziedziczenia, a pojedyncza klasa bazowa to przepis na powrót problemów, na jakie natknąłem się przy Block. W końcu (chyba) udało mi się wydzielić część zarządzania stackami:

public class PlaceStackedEffect : PlaceEffect
    {
        public interface IStackedEffect : IEffect
        {
            List<IStackManager> managers { get; }
 
            void prepare(Species affected);
 
            IStackedEffect accumulate(IStackedEffect eff, IStackedEffect acc);
        }
 
        public interface IStackManager : IEffect
        {
            void update(IEffectStorage storage);
        }
 
        public IStackedEffect eff;
 
        public PlaceStackedEffect(IStackedEffect _eff, List<IEffect> _others, List<int> _priority = null, IEffectStorage _target = null) : base(_others.Prepend(_eff), _priority, _target)
        {
            eff = _eff;
        }
 
        public override void execute(Species affected)
        {
            foreach (IStackManager required in eff.managers)
                if (!target.ownEffects.OfType<IStackManager>().Any(eff => eff.GetType().IsAssignableFrom(required.GetType())))
                    target.ownEffects.Add(required);
 
            eff.prepare(affected);
            base.execute(affected);
 
            foreach (IStackManager manager in target.ownEffects.OfType<IStackManager>().Where(sm => eff.managers.Any(required => sm.GetType().IsAssignableFrom(required.GetType()))))
                manager.update(affected);
        }
 
        public class RemoveStackedEffect : RemoveEffect
        {
            IStackedEffect eff;
 
            public RemoveStackedEffect(IStackedEffect _eff, List<IAction> _onRemove = null, List<int> _priority = null, IEffectStorage _target = null) : base(_eff, _onRemove, _priority, _target)
            {
                eff = _eff;
            }
 
            public override void execute(IEffectStorage affected)
            {
                base.execute(affected);
 
                foreach (IStackManager required in eff.managers)
                    foreach (IStackManager manager in target.ownEffects.Where(eff => eff.GetType().IsAssignableFrom(required.GetType())))
                        manager.update(affected);
            }
        }
    }

Wciąż nie usuwa to znacznej części duplikacji... np. sumowanie damage jest analogiczne we wszystkich 3 przypadkach, ale tego nie chcę wydzielać do IStackedEffect, bo łatwo wyobrazić sobie taki efekt, który by się zupełnie inaczej agregował. Tak samo analogiczny jest kod odpowiadający za dispelowanie stacków, ale znowu: boję się powrotu problemów spod znaku Block (tj. duplikacja kodu wymuszona brakiem wielodziedziczenia). No bo co, jeśli jakiś inny efekt stackowalny będzie inaczej dispelował stacki i co więcej, tak jak w przypadku Block / Dodge, jego inne dispelowanie też winno być dziedziczone po iluśtam innych efektach?

Przyznaję, że ten projekcik jest dla mnie solidną lekcją pokory.

#mon

Afish

Jak się zastanawiasz, czy przepisywać teraz, to nie przepisuj, dzięki temu kiedyś dojdziesz do momentu, w którym będziesz wiedział, kiedy to potencjalnie należało przepisać i czy ten moment przegapiłeś, a to też jest cenna wiedza.

wnuq

Nie ma sensu przepisywać teraz, tak jak to napisał i wyjaśnił przedmówca - do tego plus dla Ciebie, że zdałeś sobie sprawę z tego, że Twój obecny kod "śmierdzi" na kilometr. Natomiast przy kolejnym projekcie, koniecznie pisz automatyczne testy. Wtedy nie tylko łatwiej będzie coś zmienić ale i trudniej będzie bezsensowny kod napisać. Zapoznaj też się z wzorcami projektowymi, pomoże uniknąć Ci to duplikacji kodu, a gdy już coś pójdzie nie tak to z technikami refaktoryzacji. Zaś jeśli jesteś ambitny to poleciłbym też książki takie jak Clean Code i Clean Architecture.

kmph
2018-07-28 17:20

Wyznaję, że zaczynam rozumieć sens podziału klas na namespace'y.

Jak głupi wsadziłem całą mechanikę rozgrywki do namespace Mechanics - w odróżnieniu od namespace Hubs (sieć) czy namespace Chat. I tak tenże namespace zawiera sobie plik Moves.cs, Actions.cs (klocki, z których budujemy Moves), Species.cs, Effects.cs itd.

W konsekwencji, na chwilę obecną, taki np. Moves.cs zawiera 1740 linii i mnóstwo małych klas, włącznie z klasą abstrakcyjną Move. Actions.cs natomiast zawiera klasę abstrakcyjną Action i klasy dziedziczące po niej, razem 866 linii.

Moje rozumowanie było takie, wszystkie te klasy są już tym połączone, że dziedziczą po tej samej klasie abstrakcyjnej, dawanie jeszcze osobnego namespace'u na to (przy istnieniu i tak klasy abstrakcyjnej) to by było masło maślane.

No ale teraz mam problem: Mam sobie public class Block : Move, który używa... public class Block : Action. Ugh. Nie można tak. Chciałoby się dać: public class BlockAction : Action, no ale to jest przecież marna imitacja namespace'ów!

No więc znalazłem sobie synonim: public class Mitigate : Action. Ale! Nie tylko Block korzysta z tej Action! Mamy przecież jeszcze ruchy analogiczne, różniące się szczegółami: Fortify, Parry, itd... Zaczyna mi brakować synonimów na nazewnictwo!

A problemów tego rodzaju zaczyna być coraz więcej. Do tego stopnia coraz więcej, że w końcu się ugiąłem i zacząłem walić nazwy takie, jak FrenzyEffect (bo koliduje z ruchem Frenzy, który kładzie ten Effect).

A przecież rozwiązanie byłoby proste: public namespace Moves, public namespace Actions, public namespace Effects, itd. Zamierzam tego dokonać, ale później: teraz zbyt się spieszę, żeby w ogóle zdążyć z projektem przed końcem wakacji... (Gdyż wbrew pozorom ten refactoring nie będzie trywialny, gdyż z powodu trzymania zbyt wielu rzeczy w jednym pliku zacząłem traktować niektóre klasy jakby były namespace'ami i w efekcie mamy public class Ablaze : Effect zagnieżdżoną w public class Fire : Type. Aby zrobić ten porządek, trzeba będzie powyciągać takie kwiatki i porozdzielać po różnych folderach.)

Problem z nazewnictwem, który się zazębia: dziwactwa języka angielskiego. W tej konkretnej kwestii namespaceów jest to problem teoretyczny: np. co gdybym miał klasę abstrakcyjną Sheep i dziedziczące po niej różne rasy owiec? Jak wtedy nazwać namespace? Sheep? Pójdzie, ale zdaje mi się, że lecą warningi kompilatora wtedy. To co, Sheeps? Ugh. Wali po oczach równo, ale lepszego wyjścia nie widzę.

Zresztą na podobny problem się już natknąłem w praktyce. Otóż mam sobie public class Attack : Action. No i mam także efekty dziedziczące po interfejsach IOnAttacker, IOnAttack, IOnAttacked. Jak na razie pasuje. Ale! Mam także public class Hit : Action. I podobnie, mam interfejsy IOnHitter, IOnHit, IOnHit.... No właśnie. Past Participle dla Hit jest po angielsku identyczna z Infinitive, która z kolei jest identyczna z rzeczownikiem.

Musiałem więc być kreatywny: IOnHitter, IOnHit, IOnHitted.

Wali po oczach.

Ale trudno.

#mon

kmph

@kamillapinski: ? Jeśli piszemy Actions.BlockAction to czemu w ogóle nie zarzucić namespace'ów i nie pisać po prostu BlockAction? Z drugiej strony, jeśli to rzeczywiście takie ważne, to czemu po prostu nie importować w ciemno i nie pisać za każdym razem Actions.Block?

kamillapinski

Bo Block to nie to samo co BlockAction. Wyobraź sobie, że masz otwarte w kartach 2 pliki zawierające klasy w dwóch przestrzeniach nazw, ale nazywające się tak samo. Np. Actions.Block i Objects.Block. Gwarantuję, że prędzej czy później gdzieś się pomylisz i zaczniesz edytować zły plik. Namespaców nie warto zarzucać, bo: 1. pozwalają na logiczne grupowanie klas, 2. pozwalają na uniknięcie wieloznaczności przy korzystaniu z bibliotek (np. System.Collections.Generic.List vs My.Collections.List). Punkt 1 działa tylko wtedy, jeśli pliki w jednym namespace są w tym samym folderze.