aaronvark / PeerReview1819

Repo for peer review assignments for year 2 development class of 18/19
0 stars 0 forks source link

Reinier Maartense - Eindopdracht blok 1 #19

Closed reinier303 closed 4 years ago

reinier303 commented 5 years ago

Dit spel - Avalanche in 3D(+ more twists maybe) https://spele.nl/klim-omhoog-spel/ Er vallen random blokken naar beneden en je moet de toren die hieruit ontstaat beklimmen. Dit terwijl er een threat(mogelijk lava) druk creëert waardoor je snel moet blijven klimmen. er is een highscore voor de hoogte die je bereikt. UML: https://drive.google.com/file/d/1U1KmpHL_ilFPoKDTcYjA9CTqzrAymMgd/view?usp=sharing

aaronvark commented 5 years ago

Je UML ziet er qua opzet prima uit. Ik begrijp de pijlen tussen PlayerMovement en CameraController alleen niet echt. Qua relaties zijn er ook een aantal zaken die niet per se nodig zijn. PlayerMovement gebruikt uiteindelijk alleen de CharacterController, maar gaat om de een of andere reden ook een game manager instance en bijbehorende player instance oproepen en opslaan. In een Unity context kan die hele routing worden vermeden door de PlayerMovement aan datzelfde object te koppelen, en simpelweg een [RequireComponent(CharacterController)] toe te voegen. Misschien is er een reden dat dit niet kan, of dat je dat liever niet doet.

Je bent nog wat inconsistent met de conventies. Je maakt wel handige groeperingen in de variabelen, dus zeker als het er wat meer worden is dat evt. een reden om de conventie te breken, maar kan je uiteindelijk ook overwegem om van die collecties variabelen weer container te maken (structs o.i.d.)

Volgens mij is de GameManager vooral een PlayerReferenceStorer en ScoreAdder, want het lijkt verder weinig te managen ;).

De Spawner / ObjectPooler relatie zit wel ok in elkaar, maar is het vaag dat als de Spawner "SpawnFromPool" aanroept, die daar vervolgens niks mee doet (bijv. als er null wordt gereturned). Ook ben ik altijd skeptisch of zo'n "centrale pooler" een goed idee is, omdat je hier alles bij elkaar brengt wat niet per se nodig is. Je kan ook een pool class hebben die bijv. in de spawner worden aangemaakt en gemanaged, zonder dat je op een later punt steeds alle pool info noodzakelijk op 1 plek correct moet hebben staan (wat weer version conflicts kan opleveren als verschillende mensen aan de implementaties van verschillende pools werken). Gewoon iets om over na te blijven denken.

reinier303 commented 5 years ago

Momenteel heb ik:

Ik wil nog toevoegen:

aaronvark commented 5 years ago

Ik heb het even opgezocht, maar ik zou in de gevallen waar je met collision functies en tags werkt altijd de "CompareTag" functie gebruiken. de tag variabele returned namelijk een string waar geheugen wordt gereserveerd, en dit is in principe "garbage" die direct opgeruimd moet worden. De functie doet dit niet, en zal dus beter performen op termijn. Uiteraard is dit vooral van belang als je het veel doet, of op een performance constrained platform gaat werken (mobiel). Gebruik ook een else if als je meerdere van dit soort dingen doet, want als er eentje passed moet je de rest ook niet checken.

Tim gebruikt overigens eenzelfde soort ObjectPooler, dus als je hebt bedacht wat je daarmee wilt doen: vergelijk jullie approaches eens, en kijk of je samen een stapje verder kunt komen.

Ook zou ik toch nog eens samen willen kijken naar hoe je references deelt, omdat je volgens mij nog wel wat kan experimenteren met hoe je dingen aan elkaar linked. Nu gaat de player via de manager, OK, maar volgens mij is dit niet overal nodig. Ook zou de addscore functie een global event (public static delegate instance) kunnen zijn (zoals in Valentijn's voorbeeld) waar een score beherende class (bijv. de manager) zelf naar luistert. (goede kans om een delegate uit te proberen!)

Interfaces zal je vooral kunnen gebruiken als je dus wat meer gaat experimenteren met hoe je references verdeelt (want dan wordt die communicatie flow ook duidelijker, en kan je met interfaces daar meer sturing op geven).

reinier303 commented 4 years ago

Documentatie: https://docs.google.com/document/d/1WFK_hGwFTuk_iP9vl86UbH_UtCcKzjnv8iYQvxIeLnM/edit?usp=sharing

reinier303 commented 4 years ago

Github repo: https://github.com/reinier303/KernModuleGameDevReinier.git