kmph
2019-02-02 16:15

Jednak mam pomysł, jak można by rozwiązać bolączki z frontendu opisane w poprzednim poście w temacie #mon .

W tym projekcie, co się uparłem pisać kod po polsku, też robiłem front. Tam jednak postanowiłem wyciągnąć wnioski z tego kodu, który opisuję w niniejszym poście, a który szczerze mówiąc naprawdę mi już zbrzydł. Myślę, że z zadaniem uporałem się całkiem nieźle: choć dalej pisałem w czystym JS, jednak naprawiłem większość (nie wszystkie, o tym za chwilę) problemów z "moim JS".

Mianowicie zacząłem ostro wykorzystywać dobrodziejstwa nowego EC6. Zamiast jednego wielgaśnego pliku .js dałem wiele mniejszych, załączanych za pomocą import. Co ważniejsze: zamiast gigantycznych, wielokrotnie pozagnieżdżanych funkcji miałem klasy, z których każda odpowiadała za jeden fragment strony. (Klasy te implementowały zatem HTMLElement, myślę, że własne tagi to kolejne wielkie dobrodziejstwo EC6). Zamiast zatem mnóstwa globali i pseudo-globali, które wymagają ogarniania całości kodu, by wprowadzić najmniejszą zmianę, miałem tam obiekty, które mogły już implementować polecenia takie jak: "user się wylogował, obsłuż to". No i dzięki temu już nie trzeba przebudowywać połowy strony, by wyszarzyć dwa przyciski. No i chyba dwa teambuildery obok siebie nie byłyby strasznym problemem: po prostu budujemy dwa customowe tagi my-teambuilder i wszystko (prawie).

Wydaje mi się, że skok jakościowy jest ogromny. Jednak mimo to, jeden problem pozostaje nienaprawiony, a jeszcze inny jest nawet wprowadzony.

Mianowicie, jak by taka architektura wyglądała, gdyby przełożyć to na tę grę, o której piszę teraz? Załóżmy, że user sie wylogował. No to teraz obiekt w przybliżeniu odpowiadający za całą stronę przepycha tę informację w nieskończoność w dół: najpierw do obiektu chatbox, potem do obiektu chatbox-kontrolki, który to obiekt wreszcie wyszarzy przycisk i zamieni napis Send to chat na Log in to speak in chat. Wow, tylko trzy poziomy? Jestem pewien, że byłoby tego więcej. Ale za to mamy jeszcze drugą gałąź: najpierw do obiektu panel główny, potem do obiektu odpowiadającego za wchodzenie do matchmakera... itp. Koniec końców, wiele klas implementuje analogiczne funkcje tylko po to, by przepychać jakąś informację niżej w hierarchii. No i wyżej: Wszak przycisk "wyloguj" też musi przepchać kilka poziomów wyżej informację, że został naciśnięty. To są copypasty.

Inny problem - ten nierozwiązany - z copypastami: Chodzi mi o takie fragmenty:

let span = document.createElement('span')
span.className = 'error'
span.appendChild(document.createTextNode('You still need to choose ${n} mons for your team')
div.appendChild(span)

To się powtarza w kółko i w kółko. W obu projektach. Nie mam pomysłu, co z tym zrobić. No w sumie można by tak:

function appendErrorSpan(div, message) {
    let span = document.createElement('span')
    span.className = 'error'
    span.appendChild(document.createTextNode(message))
    return span
}

Ale po pierwsze: Ile takich funkcji tego rodzaju (i to globalnych!) musiałoby być? Pierdyliard. Po drugie: I tak one wszystkiego nie obsłużą: co, jeśli np. teraz trzeba dodać na początku tego spana jakiś przycisk? Albo spana w spanie, przy czym zagnieżdżony span ma własne id?

Ja myślę, że to by się dało załatwić. Np. gdybym się bardzo postarał, może zrobiłbym tak, bym mógł pisać takie rzeczy w ten sposób:

   div.appendContents([
        {type: 'span', class: 'error', contents: [
            {type: 'button', onclick: 'cośtam', text: 'cośtam2'},
            {type: 'textNode', contents: 'cośtam3'}
        ]}
    ])

Ale to w zasadzie musiałbym napisać własny ogromny framework do JS? Zresztą (poprawcie mnie, jeśli się mylę) duplikujący funkcjonalność JQuery? Nie piszę się na to. Chyba na razie zostanę przy copypastach, a potem zacznę się uczyć JQuery ;/

No ale. OK, skończyłem projekt "polskojęzyczny", jestem mądrzejszy, wracam do swojej gry... i chcę kontynuować tworzenie teambuildera... no i co mam zrobić? Pisać tak, jak bym chciał? Może by należało, ale już cały kod jest napisany tak, jak jest... no pisałem tak, jak dawniej, chociażby po to, by spójność zachować. Zresztą zalążkowy kod teambuildera już był, nie chciałem wywalać całej funkcji 'prepare_teambuilder', a tego wymagałoby wykańczanie teambuildera porządniej. Pewnie niesłusznie. Teambuilder (mimo, że połowa tambuildera była napisana później) jest więc tak samo brzydki, jak reszta kodu.

Pocieszam się chyba jedynie tym, że kod JS jest w zasadzie w całości do wywalenia, prędzej czy później. Tym bardziej nie chce mi się poprawiać go systematycznie. Albowiem, czy to w ogóle powinien być JS? Prędzej czy później wyjdzie chyba wreszcie finalna wersja Blazora, docelowo chyba byłby sens napisać front w Blazorze właśnie. Co i tak będzie wymagało wyrzucenia całego kodu JS. Po co więc go poprawiać?

Inna sprawa, że front i tak mi zjechaliście od góry do dołu ;P Na brak wyczucia artystycznego nawet Blazor nie pomoże, trzeba będzie siedzieć nad kolorkami w nieskończoność, pewnie kicz mi wyjdzie niewiarygodny, ale to już na później robota. Teraz balans i bugi.

vpiotr

Wygląda trochę jak jakieś demo w Scratchu.

kmph
2019-02-02 16:14

Praca nad #mon idzie, choć mocno powoli... Ale idzie :)

Dwa dni temu czy coś koło tego skończyłem robić teambuildera. Teraz czas się zabrać za bugi i balans, obawiam się, że w szczególności to drugie może być dużo trudniejsze, niż z początku przewidywałem.

Ale na razie jeszcze w temacie frontendu, bo przecież teambuilder jest częścią frontu.

Kiedy zaczynałem pisać ten interes, JS znałem zdecydowanie gorzej, niż C#. Z tego powodu pisząc frontend świadomie postanowiłem... nie dbać zbytnio o jakość kodu. Backend w C# już starałem się, jak na swoje umiejętności, pisać w miarę ładnie: wyszło jak wyszło, o czym we wcześniejszych wpisach w temacie, no ale załóżmy, że jak na mnie to jest znośnie. Zresztą zamierzam spróbować wyczyścić kod backendu prędzej czy później.

Jeśli chodzi o front... no cóż, dość powiedzieć, że od początku jedynym moim zamiarem było napisać go tak, by po prostu działał.

Zresztą front pisałem w czystym JS: bez żadnych JQuerych, Angularów, Typescriptów i innych bibliotek. Powód był ten sam: świadom, że JS znam marnie, uznałem, że należy podejść do nauki od fundamentów, a nie od wyższych kondygnacji: tj. bibliotek zacznę uczyć się dopiero, kiedy będę wiedział, że i tak wszystko będę w stanie napisać w czystym JS. Z tego samego powodu pisałem także w czystym CSS: bez żadnych Bootstrapów ani innych podobnych pomysłów.

W konsekwencji front się obecnie w olbrzymiej większości sprowadza do: jednego pliku index.cshtml na dokładnie 170 linijek, jednego pliku .js na 1686 linijek (który buduje po kolei olbrzymią większość tagów HTML: w zasadzie jest niekonsekwencja, no bo czemu nie wszystkie?) i jednego pliku .css na 800 linijek. Jest to i tak sporo mniej niż ilość kodu w C#.

Gdy zaczynałem pisać front, nawet nie wiedziałem, jak się robi prototypy w JS. W konsekwencji nie ma tam żadnej obiektowości. Są natomiast wielkie funkcje na setki linii, takie jak prepare_chat, które w pewnym sensie imitują klasy: wszak zadaniem prepare_chat jest zbudowanie i powiązanie wszystkiego, co odpowiada za chatbox (włącznie z kontrolkami do mute'owania i banowania userów dla adminów). W funkcjach tych zatem będzie wiele pseudo-globalnych zmiennych (w sensie: zmiennych niby scope'owanych do funkcji, ale notorycznie wykorzystywanych przez onclicki czy inne onsubmity i oninputy w elementach budowanych przez tę funkcję - w konsekwencji wprowadzenie zmian na samym dole ogromnej funkcji wymaga dokładnego ogarniania całości jej działania), wiele (czasem wielokrotnie) zagnieżdżonych funkcji (funkcja do zbudowania pola bitwy zawiera funkcję, która buduje konsolę, która zawiera dwie zagnieżdżone pętle obsługujące battle updates (pierwsze zagnieżdżenie: tury, drugie zagnieżdżenie: update'y na turę), w pętli tej jest ogromny switch z case'ami na każdy rodzaj update, a w środku jeszcze ify... Naprawdę nie wiem, jaki maksymalny poziom zagnieżdżeń udało mi się osiągnąć, ale na pewno co najmniej 8(!))

Z jednej strony można powiedzieć, że nawet nieznajomość klas w JS nie usprawiedliwia czegoś takiego. W sumie zgodziłbym się. Ale z drugiej strony... Jeśli jedno logicznie należy do drugiego, (jak np budowanie konsoli należy do budowania pola bitwy), to czemu nie wykorzystać zagnieżdżania funkcji, zamiast zaśmiecać przestrzeń globalną?

Zresztą przestrzeń globalna też jest zaśmiecana. No bo tak: załóżmy, że user się wylogowuje. Należy powprowadzać zmiany na stronie. Co gorsza, zmiany nie w jednym miejscu (np. tylko w logowajce), ale w zasadzie drobne zmiany wszędzie: a to trzeba wyszarzyć przycisk wchodzenia do matchmakera, a to należy wyszarzyć pisanie w czacie, a jeszcze corner case'y... A właśnie, teraz mi się przypomniało, co jeżeli user się wyloguje podczas trwania bitwy? Czy mam to obsłużone? Nawet nie wiem prawdę mówiąc, muszę sprawdzić) W podobnych sytuacjach mam do wyboru: albo wszystko zbudować od zera, albo poza daną funkcją wprowadzić poprawki w tym, co ta funkcja buduje. W zasadzie w większości wprowadzam poprawki wtedy, kiedy zmiany wchodzą w "zakres kompetencji" więcej niż jednej ogromnej funkcji (np ww logowanie), natomiasat buduję niepotrzebnie od zera wtedy, kiedy konieczne zmiany zawierają się w pojedynczej ogromnej funkcji (np. w teambuilderze obok każdego mona wyświetla się podsumowanie jego statsów oraz dostępne dla niego ruchy - te wybrane są wytłuszczone; no to jak user wybierze kolejny ruch, to przebudowujemy CAŁE podsumowanie, zamiast tylko wytłuścić pojedynczego spana; itd, itp)

-- a nie, chyba jednak wylogowanie przebudowuje niemal całą stronę... To ja już nie wiem, nie do końca pamiętam , co robiłem trzy kwartały temu... Ale na pewno są jakieś durne funkcje globalne odpowiadające za wprowadzenie zmian gdzieś bardzo nisko. Chyba nie ma konsekwencji, kiedy przebudowujemy, a kiedy wprowadzamy zmiany.

Nie wspomnę jeszcze, że te wielkie funkcje komunikują się ze sobą po zmiennych globalnych (bo jak inaczej?) oraz o kwiatkach tego rodzaju (dobra, jednej biblioteki użyłem, tam gdzie nie dałbym rady zrobić samemu, czyli jak muszę graf rysować):

    g.setEdge(monsAndMoves.moves[monsAndMoves.mons[monname].moves[i]].prerequisites[j], monsAndMoves.mons[monname].moves[i], {
        class: `teamBuilderMoveEdge${i}-${j}`
    })

Co się tu stało? Ano monsAndMoves to jest obiekt powstały ze sparsowania JSONa przesłanego przez serwer. Jak głupi pozostawiłem ten obiekt jako globalny bez naprawienia referencji, a potem naprawiać tego nie chciałem, bo nie chciałem szukać i poprawiać wszystkich miejsc, gdzie ten globalny obiekt jest użyty. Gdyby referencje były naprawione, to powyższy potworek wyglądałby tak:

    g.setEdge(monsAndMoves.mons[monname].moves[i].prerequisites[j], monsAndMoves.mons[monname].moves[i], {
        class: `teamBuilderMoveEdge${i}-${j}`
    })

Mimo wszystko postęp, prawda? Oczywiście powinno to raczej wyglądać tak:

    g.setEdge(currentMon.moves[i].prerequisites[j], currentMon.moves[i], {
        class: `teamBuilderMoveEdge${i}-${j}`
    })

Ale w tym celu musiałbym w miejscu, gdzie definiowany jest monname, zdefiniować także currentMon jako monsAndMoves.mons[monname] - co nie jest w sumie trudne - ale i za każdym razem, gdy monname ma przypisywaną nową wartość, także przypisywać nową wartość currentMon - dziękuję pięknie nie... (oczywiście monname jest globalem)

O tym, że często nie mając pomysłu na to, jak przekazywać referencje do elementów HTML, biorę sobie je w kodzie za pomocą powtarzanego po pierdyliard razy document.getElementById('cośtam cośtam') to także nie ma sensu wspominać. Ani o tym, że o string template'ach nauczyłem się dopiero potem, w związku z czym mamy takie kwiatki:

    let title = 'Power: ' + monsAndMoves.mons[mon].stats.power + ' Speed: ' + monsAndMoves.mons[mon].stats.speed + ' Type(s): '
    for (let type of monsAndMoves.mons[mon].types) title += type + ' '
    name.title = title

Jeszcze inny problem: Załóżmy, że bym chciał wprowadzić nową funkcjonalnośćfunkcję, np taką, by można było mieć dwa teambuildery otwarte obok siebie, każdy edytujący inny team - to co? Obecnie nie wiem, jak, właśnie ze względu na to, że team oraz teamInBuilding to są globale.

Pomysł rozwiązania tych bolączek w następnym poście :)

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