Mark Meerssman - Eindopdracht, Iteration 1

[Markreel]

Een Retro Game met een twist, concept zal spoedig volgen.

[aaronvark]

Ik zou voortaan de uitgebreidere description juist in de issue plaatsen, ipv de pull request. In de PR kan je dan vooral de focus leggen op wat er in deze iteratie is bijgekomen (ook vooral gericht op andere studenten). In de issue kan je vervolgens meer richting het Valentijn, Yassine en ik broadcasten.

UML is prima overzichtelijk. Zorg er wel voor dat je de volgorde +, #, - aanhoudt (in de Lemming class staat er onderaan nog een extra public).

In de lemming class ben ik ook wel benieuwd waarom je er voor kiest om alles private SerializeField te maken. En in die class zie ik ook een nested ternary operator: "velocity = new Vector3(onXAxis ? (dirRight ? 3 : -3) : 0, 0, onXAxis ? 0 : (dirRight ? 3 : -3));"... over het algemeen komt dat de leesbaarheid niet ten goede (en volgens mij banned Ronimo hem sowieso, nested or otherwise). Waarschijnlijk is het leesbaarder om, als het toch een paar checks zijn, die even in een functie te embedden en dan daar een goede naam aan te geven. Scheelt evenveel regels code, en je hebt geen editbug-gevoelige, lastig leesbare regel.

De Grounded functie daar heeft ook hele lange regels met daarachter nog een verstopte "returnValue = true;", dat moet overzichtelijker kunnen schat ik. Hier evt. ook weer de code die veel op elkaar lijkt (die raycasts met allerlei lange variabelen) in een functie wrappen om het korter en leesbaarder te krijgen.

Overall best netjes, maar duidelijk WIP (commented code hier en daar), en waarschijnlijk zal je in je achterhoofd kunnen houden of de Lemming class niet te veel responsibilities krijgt na verloop van tijd.

[Markreel]

Ik heb de feedback op mijn vorige commit toegepast en mijn code in het algemeen opgeschoond. Ook heb ik voor mijzelf een repository aangemaakt waar ik mijn volledige project op bijhoud. Ik wil het graag nog hebben over ternary operators en wat de voor en nadelen zijn van het gebruiken van ze, dit heb ik ook in de code aangegeven met een //DISCUSS.

Achter de //DISCUSS licht ik toe waar ik het over wil hebben, is het gebruikelijk om dat te doen of maakt dat het slordig?

Deze versie van mijn Class Diagram is ook te vinden in het mapje "Documentation".

Repository van het gehele project

[aaronvark]

Goed om die //DISCUSS te gebruiken. Wel zal dat vooral zin hebben in projecten waar je samen programmeert, omdat iedereen ze dan tegenkomt. Aangezien ik niet in je project werk is de kans iets kleiner dat ik hem zie, dus goed dat je hem ook benoemd.

Qua wat de voor/nadelen zijn is het vooral leesbaarheid. Als je hem niet nested gebruikt vind ik het meestal niet zo'n issue, maar je zou voor het experiment het volgende eens kunnen proberen:

• Maak een nested ternary operator die een belangrijke check uitvoert.
• Maak een non-ternary variant van exact hetzelfde gedrag
• Introduceer in beide dezelfde bug (bijvoorbeeld een verkeerde check, of de true/false code omdraaien)
• Vraag aan verschillende mensen om te kijken wat er mis is, door alleen de code te lezen. Kijken of het bij de ternary operator langer duurt (ik verwacht van wel).

Dit gaat ook weer helemaal over "optimize for time". Ook zou ik geen ternary gebruiken waar hij niet nodig is, bijvoorbeeld in "IsDimensionalLemming". Daar kan je ook gewoon zeggen "return ... == ...", zonder de ternary. Doet hetzelfde.

Probeer ook GetComponent calls zo min mogelijk te doen. In de OnMouseDown heb je er nog een paar staan (naar renderers) die volgens mij gecached kunnen worden.

Verder goede voortgang!

[Markreel]

Ik heb sinds de vorige update voornamelijk de visuals van het spel geüpdate. Ik heb een model gemaakt voor de lemmings en heb ze simpele animaties gegeven. Ook hebben alle materials in de scene een Toon shader om het spel wat meer stylized te maken.

De speler kan nu ook lemmings zelfmoord laten plegen (heel grimmig). Ook worden er nu een hoop stats bijgehouden over de lemmings die via de UI worden weergeven.

Hiervoor heb ik nog nooit een Interface gebruikte en nu gebruik ik ze voor mijn Singletons!

[vmuijrers]

Je game ziet er goed, maar houd er rekening mee dat we je daar niet op beoordelen :) Je kunt nog een ObjectPool schrijven voor je Gems, ipv ze te destroyen wanneer ze worden opgepakt. Daarnaast zie ik dat je een switch-statement gebruiken voor de states van de lemmings, echter beperkt dit de abilities van de lemmings, het combineren van abilities op een lemming is nu uitgesloten. Tevens scaled je code nu niet zo lekker, want hoe meer abilities je toevoegt hoe groter de lemmings-class wordt en hoe groter de switch-statement. Je kunt dit op meerdere manieren oplossen (misschien te veel voor deze opdracht, maar wel goed om te realiseren dat Switch-statements bijna altijd onnodig zijn). Je kunt de abilities in hun eigen class zetten en deze opslaan in een lijst per lemming, zodat je in de update van de lemming de verschillende abilities af kunt lopen zodat een lemming meerdere abilities kan hebben (denk aan een parachute geven terwijl hij nog moet graven oid). Wanneer je de abilities in hun eigen class zet, zijn ze tevens overzichtelijker en modulairder. Om conflicten tussen volgorde van abilites te voorkomen kun je ze een Priority Property geven en hierop sorteren voordat je door de lijst van abilities heen loopt (indien nodig).

[Markreel]

Link naar de repo: Github Repository