aaronvark / PeerReview1819

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

Casey Hofland - Pacman without the Maze #14

Closed CaseyHofland closed 4 years ago

CaseyHofland commented 5 years ago

Het is Pacman maar zonder, well, the maze. Het is gewoon een doos waarin je 360 graden kan draaien om alle pallets te eten. De ghosts: Blinky, Pinky, Inky en Clyde, hebben allen hun zelfde behaviours, slightly aangepast, als in de originele Pacman. Het spel gaat door tot de speler 3 levens is verloren. Wanneer de speler een level haalt, gaat hij door naar het volgende level, wat precies hetzelfde is alleen zijn de geesten iets sneller.

Het idee is om dit spel zo snel mogelijk te maken. Daarna, kan er gekeken worden naar extra variaties en verbeteringen op het spel (e.g. de oogjes van de spoken volgen de speler, powerups, betere behaviour door middel van Nav Meshes, etc.)

aaronvark commented 5 years ago

Heb de code op je branche wel even bekeken. Zorg dat je nog in een pull request een reference naar deze issue #14 plaatst.

Je gebruikt een aantal keer get => in een property, maar volgens mij kan dit ook zonder het gebruik van een property. Dus gewoon direct, "variable => ...", dan maakt C# daar intern ook direct een get property van.

Ik zie ook inderdaad je IScore interface terugkomen op dezelfde manier, maar in hoe het nu geïmplementeerd is lijkt de interface niet zo veel toe te voegen (nu is de score manager het aanspreekpunt, en kunnen classes die iets met score doen daar gewoon bij, en regelen ze zelf hoe het werkt).

Ben ook wel benieuwd naar hoe je de OnEvent class, uit je framework, over het algemeen gebruikt, en waarom je het zo hebt opgebouwd.

Overall is het allemaal best netjes opgebouwd, maar kost het nog wel wat moeite om een goed overzicht te krijgen van hoe dingen in elkaar hangen. Wel hier en daar interessante toepassing van language features.

CaseyHofland commented 5 years ago

Beste Aaron,

Ik heb het gedaan - sorry voor er nu pas op terug te komen.

Ik wist niet van die shortcut; heb hem gelijk geimplementeerd, zoiets gebruik ik vaak voor dynamische bools!

De IScore is nog niet goed geimplementeerd omdat ik er nog geen ervaring mee heb. Origineel wilde ik een IScore naar de ScoreManager sturen, maar ik kon er geen referentie van krijgen in de class zelf (behalve met GetComponent<>, maar dat vangt niet de case op van "Wat als deze component 2 IScores heeft?)

De OnEvent class kan je zien als een algemene trigger en is ontzettend handig om snel iets connecties op te zetten zoals bijvoorbeeld Buttons dat doen. Ik gebruik hem bijvoorbeeld om op Pellets Consume aan te roepen wanneer pacman hun trigger entered. Deze heb ik gemaakt omdat ik tijdens project vrij met iemand zat die constant scripts wilde met nieuwe hele eenvoudige triggers voor "een animatie" of "een object vernietigen", en toen heb ik deze class ontworpen zodat hij gewoon Unity's accessible functies kan aanroepen en hij 90% zelf kon implementeren.

Sorry dat het overzicht slecht is, het komt vooral omdat ik probeer om alle code uniek voor dit spel los te koppelen voor code die ik voor elk spel kan gebruiken, en dat 2de krijgt bij mij voorrang waardoor connecties soms een beetje vaag kunnen lijken.

Met vriendelijke groet, Casey.

Op do 5 sep. 2019 om 15:56 schreef Aaron Oostdijk <notifications@github.com

:

Heb de code op je branche wel even bekeken. Zorg dat je nog in een pull request een reference naar deze issue #14 https://github.com/aaronvark/PeerReview1819/issues/14 plaatst.

Je gebruikt een aantal keer get => in een property, maar volgens mij kan dit ook zonder het gebruik van een property. Dus gewoon direct, "variable => ...", dan maakt C# daar intern ook direct een get property van.

Ik zie ook inderdaad je IScore interface terugkomen op dezelfde manier, maar in hoe het nu geïmplementeerd is lijkt de interface niet zo veel toe te voegen (nu is de score manager het aanspreekpunt, en kunnen classes die iets met score doen daar gewoon bij, en regelen ze zelf hoe het werkt).

Ben ook wel benieuwd naar hoe je de OnEvent class, uit je framework, over het algemeen gebruikt, en waarom je het zo hebt opgebouwd.

Overall is het allemaal best netjes opgebouwd, maar kost het nog wel wat moeite om een goed overzicht te krijgen van hoe dingen in elkaar hangen. Wel hier en daar interessante toepassing van language features.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/aaronvark/PeerReview1819/issues/14?email_source=notifications&email_token=AGTSAQ2PJMV3OYIDCN4XWGDQIEFXTA5CNFSM4IS5OWWKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD57GGZQ#issuecomment-528376678, or mute the thread https://github.com/notifications/unsubscribe-auth/AGTSAQ5ROK5NVHZH6NGZ3CLQIEFXTANCNFSM4IS5OWWA .

aaronvark commented 5 years ago

Goed om te zien dat je de Unity statemachine ook aan het proberen bent! Ben benieuwd hoe je ervaringen daar uiteindelijk mee zijn, en wat je er handig / minder handig van vindt. Interessante manier om de score manager met de interface los te houden van dingen die score toevoegen

Nette inzet van de Movement2D als onderdeel van de Player en Ghost class. Een dergelijke Composition is meestal flexibeler van het via base classes inheriten, en de Movement2D class is klein en focussed genoeg om herbruikbaar te blijven.

Zijn er nog specifiek dingen die je structureel wilt proberen? Morgen is er tijd om daar nog over te sparren.

CaseyHofland commented 4 years ago

Ultimate Pacman Documentatie.pdf