Unity Code Review — Refactoring Some Common Unity Mistakes

so this piece of code I just want to say who it was from was from direct see on the discord server so let's give it a look-see hmm you see my screen there Jason yeah all right so we're pretty much looking at this for the first time I did pull it up and paste it and just kind of gave it a glance but I guess I think the more of these we do will probably come come up with like a step by step process to looking at code but just off the bat I can see writer is telling me a couple things you brought this up earlier yes what's this blue line say repeated property access a built in component is inefficient that's that call to this dot transformed our rotation yeah now it's not as bad as it used to be so I will put a copy out there and so when they architected the the kind of programming and unity in the past they wanted to simplify a lot of concepts and so one of them is that unity is built on a component model so behind the scene do you only think about it when you see mana behavior a behavior is a thing that exists on an object and an object it's multiple behaviors and you kind of Intuit this as you use unity but you don't even think about it as an object is a list of behaviors and so what happens is every time you write an innocuous statement like doc transform or dot rigid body or dot renderer any of them you have to basically search the object and find the one that you're looking for and so if you write this stuff transform dot this that transform dot every one of those constitutes a small search now it's not a big search and unity if memory serves caches some of these now it never did before but it's some of them the cache now transform might be one of them and I'm not entirely sure but to be safe I still like to cache the transform before I do a lot of work on it I was gonna I had looked myself it goes to the C++ see it's not gonna be able to see the actual and see the call fortunately yeah and again same goes for the camera I think we've mentioned that before camera dot name that is the horrible call to game object at find object with tag main have any of those in here no but yes illegal it'll actually have to search for it doesn't have the main camera tag it'll throw a little exception and if it does have the if it does have it you're still doing a search to find it yeah so long story short if you're if you find yourself writing they start transformed out there start transform that transform dot even if it's just in the update you put a transform catch it once and then use it for the duration of what you're doing so we should be able to refactor this let's see let's see if I can do it one shot there's also one other thing to keep note of there is something called transform on the object obviously for that we recommend it so cache it with the same naming convention of underscore or something that effect wait ok I true that I'm so no I like to see that there is underscores as a little tip for people who aren't familiar that in the copy back on this core is recommend it's is a indicator that it is a class scoped object so it lets you know this thing is a property that exists on this class so it's very useful to say this Jenga piece has an original position if it's not using another score it's saying this is a property I'm using internally to work with stuff to calculate some other value alright so we successfully encapsulated that though I don't know if you guys saw that but I was able to hit all eight occurrences of one shot thanks to writer oh that was nice well I think you've spoken about this well first of all I'm gonna kill this comment because come on really come on yeah by the way laughs please go in and have a look at how to change the template that you can use from other papers yes so if there's I got a video somewhere and I think I'll link to it somewhere that you can even search online you can change the template that unity will use to generate new c-sharp minor' behaviors so change that file and choose how you want to look get rid of those comments or make a video about that oh yeah I have one from two years ago no no we need a 20 by the time I make it a 2020 video I don't date a refresh and that old video doesn't have my beautiful acting skills so we'll see all right so what do you think about this I noticed that he doesn't explicitly put the private modifier in front of these I think all yeah I don't like it because I think in my mind every new word is another it's a piece of information it's kind of whether you're saying every keyword should add value right every word you're adding is going to be there to elucidate some point about an object and private is telling you every single item in this thing is private and that's fine but it's an unspoken rule of everything in programming that it's private unless it's not so you could let C code and groovy oh yeah I'm not even kidding you everything is public by default I mean horrible wrote that language I'm sorry I think that can be a random at least you've got a rat this time that's good yeah so first thing I don't use them I don't use the pre private simply because if you can into it if you use the language you know it's already private so it's not adding new information it's just the more words to look at your crop so the personal preference it's not a be all end all it's just I don't use it yeah my personal preference is to be explicit a very good point we talk about teamwork right here if you're on a team hmm hey it's standard that everyone uses and stick to it I don't even know that's not my personal style if I start doing half of my code not using it and he's using it that way you end up with a really horrible mess of confusion now granted you can you can have you know style copper's was mentioned in the chat as an option that'll automatically do this for you but you don't get into this weird position where your code has been have for it and automatically by a stack up just you know be a good citizen as it's called and follow whatever the convention is anyone else is using and so in this case we're using the privates I think yeah I think is useful you know when when it's like really obscure styling like if you're on a team that's just like so used to writing like legacy code or something and it's just as obscure like you know it's it's nice to have that just to kind of keep you honest but yeah you don't want to have to depend on that yeah all right so next up I guess that's all that's all pedantic like I wouldn't be too worried about stuff we said up to this point here we're getting to our first actual worry or first thing to really think about and that would be our good friend here well actually no we'll do one thing first which is right now that little green line on the if statement before we get to that one the if statement the green line this isn't mandatory but it's another bit of good practice right that's going to tell you you can invert it that's what that green line will say so if you if you go over to that if and expand it out it will tell you invert if and it does this now what this does is it tells you that you had an if statement where there wasn't the else portion so what you could do is it turns into a guard clause you're basically saying I can take this and I can just exit out because nothing else runs past this point if this is not true or in this case if it's the opposite of what you're doing if you shouldn't dissemble then ignore don't do anything else beyond this point and that's just a bit cleaner it lets you exit sooner and you don't end up with what's called cyclomatic complexity and that's where you have multiple brackets deep and all of a sudden you have to mentally kind of zigzag your way through your code so if you if you exit early you can work on cleaner code step by step you know yeah so yeah this guard Clause is great I mean obviously you want to make sure so for me the having the guard clause in the up in the update method I always feel a little bit like well the update method you might you might update it later huh you might change it later and so the update method could do so much more that you you can't predict because it's so generic so I always like kind of I don't want they shy away but it always makes it always feel like a code smell to me when I see it when I when I put a guard clause into an update but I mean I think in this case well I fun fact actually bring it up I do something about that myself which is whenever I have a guard cause what I do is I wrap everything else in the update and I give it a name so I would in this case call this update assembling or or I put it and then you then you have a guard clause and then the function that's called and that way you could very clearly see what would happen in the case of it hitting past the guard Club got it cool all right so will probably end up doing that but I'm not let's see what this thing does well you want to know talked about this and I think that's yeah this is this is the first one that everyone should really be aware of right and I say this a lot because I think we need to make a video on there especially I don't think people fully understand what a component model is and how it works and that basically means a lot of stuff is happening magically by unity to kind of do things for you least of all the update function we haven't even gotten to magic functions yet I don't want to get into that right now but this call in particular is well worth knowing because people don't think about it but what that getcomponent does like I said imagine you had a stack of books and you want to get a certain book and someone said hey will you hand me that book and then the minute they say well they'll say do you have this book and you would then search the list get the book and say yes I have this book then you put it back in the stack and then someone says can I have that book you would then go through it again and find the book and handed to them you're doing the same thing twice arbitrarily that's costing extra cycles so whenever you call it get component you want to store the value of that so you can get back at it easier so you would remember where it is and you can refer to it so if you ever find get component inside of an update loop what you're basically saying is every time update is called walk through all the objects and then every time updates called walk through the objects again and some people might say there's only one or two components on this object it's not that much of a walk well there's one or two components now and somebody might add more later and also if you've got ten items and they're all doing a walk of only two steps that's still 20 steps you know like you're you're massively scaling that as you go so get component cache them in your wave function let's do it let's catch this baby and we'll Charles are doing that I'll point out why I said a wake function here's another interesting point based on the application model they're using a state system a life cycle that's very similar to say Android and so what I mean by that is every single script starts with there's a series of steps and there's a week and their start and then there's no them I don't get all of them but the general point is awake happens then start happens then update happens and what a lot of people don't realize is a wake and start happen once for the lifetime of that object so if we had a wake start on enable update on disable no matter if you toggle that object on and off what happens is the start will run once the awake will run once and the enable will toggle on and off so it's caching the data for it and then there's using the data as you go forward so what you want to do is anytime you want to use objects like this would get components you do in the wake and the reason you do an awake instead of start is every single awake call in your entire application runs before the first start call so if you've got 50 scripts and you don't know which things would initialize first and which variables will be set what you want to do is want to make sure that all the awake calls run so all of your data is cached and then the next thing that executes will be the start function so do all of the the logical bindings in a wake do all of your configuration and start alright so we push that up I was wondering why this was complaining is because it doesn't like that I called it rigidbody like it doesn't understand that word so it'll be not probably just the type of oh yeah I'm gonna I'm just gonna add it to my user dictionary yes I didn't so they not gonna tell me what everybody is to answer the question are there hidden components there can be worth noting transform is a component so you already got one to start with but there's things called display flags and so what you can do is you can actually have updates and a good example of this is if you use any kind of hierarchy tool so say you're installing cue hierarchy or our row which hierarchy Pro or I think even master audio a few others do it what they'll do is they'll they need some object to hook into so when your application lifecycle starts so they will create an invisible object in your hierarchy and they will hide it so it's actually there but you can't see it and there's a couple of others too so you can actually write a script yourself in the editor which will iterate through the objects and toggle all the visibility flags to make them all visible if you're really curious but for the most part it just boils down to there will be some hidden objects as well so yeah that's worth knowing all right Wow could you take a look oh I thought this whole thing was the if statement I was like no so long story short with all of that we have first of all we've now patched that rigid body that's a good start positions but it would have been cool to look at the profiler to see if we in fact that is an example of sometimes you'll make a video that demonstrates the difference between call and get component update versus what caching it yeah so next up one thing that's worth and line 32 right there is setting the rigid body to kinematic that's fine but keep in mind right with the whole lifecycle concept we talked about okay happens every frame now this isn't a big deal it's not gonna cost you that much but it is going to call that every single update what you can do if you're in control of that kinematic state you're better off having some function call that sets that you know sets it to kinematic or sets it not kinematic right after what you're doing it's a state it doesn't need to be constantly updating now I don't know the context this is in so I wouldn't bother extracting it now but just keep in mind when you're designing something you should never really be constantly updating something into a state that's a boolean flag yeah it'd be interesting to see if this is even if this is even needed yeah I wouldn't say it is there's an enable physics method there so I would say that probably isn't actually been a necessary call but again it's not gonna break the bank I wouldn't be too bothered which you know also goes into the readability of this code you know it's like what is this doing why is this here there's no context and I you know I'm really big on self documented code you mean I've thought I want to sound mean but no no offensive ever wrote this but you know if you're gonna write a comment up here over the start method that says this gets called the beginning of the frame why wouldn't you know something that's fairly obvious if you're a unity developer well why wouldn't you put a comment over this tell me what it does yeah in fact actually I would even go one step further and say this is a bad idea mainly because there is a function called set kinematic which leads me to assume you can toggle kinematic on and off and because this is being retried every update that's actually a bad idea because what's gonna happen is someone will say enable physics false they're in the wait for seconds and that will immediately get reflagged back on so enable physics that's actually a bug because if you if you do this enable physics it'll immediately fail again outside of the other flag now I can guess what's happening is that you scroll up to that yeah this should assemble is meant to gate that so the is kinematic to only meant to fire as soon as should assemble but that's something we have to Intuit by going back and forth realistically what you're saying is is when that when that's true is kinematic is true so maybe that is kinematic should be in your should assemble function because that's what the two things are intrinsically tied to shouldn't be part of the update yeah we think about this uh log statement here yeah yeah one don't login updates just a bad idea but put it back for one second I wanted to put out a couple things first things first a lot of people don't know this you can actually change it just print instead of debug log that's an inherent built-in function exactly the same thing just print quotes Jack insane nice a nice little shorthand if you're doing quick tests but begin don't use it I'm just letting you know it's there does adjusted yeah I know most people know they don't use it though let me tell you is there cuz I think it's addressing what else you can do here's more importantly if you are going to do a debug log it's an often time a hard time to find where that is so write debug log again actually and we'll look at the autocomplete it's a debug log and you see there's a whole other options but the one I'm going to point out there is if you'll notice if you go to the top one there's an object context and that's really really valuable because what happens is if you write a log and you put an object in there your good example being literally this what will happen is when you get the log message and you click it in the console it'll pane in the hierarchy it'll turn yellow and you'll actually see it so it not only tells you there was a blog but it'll tell you where in your code that log actually happened from so usually you want to put the context in there for when you're logging it so but long story short episode all that don't use logs in updation and even if you are we can get into stuff later but you want to do you want to create your own logger and you wanna be able to talk on all logs off in one go you can even override the unity lager but I don't get into the complexities of that yeah oh yeah she did a video on that where I was I did a log fernette implementation which that was interesting alright so I'm trying to figure out what this this code does I think it's Jenga like you know the game Django where you pull the pieces out of the tower and it falls down I think basically it's almost like at the end of the game when everything has fallen it it sets like a flag to assemble to recently yeah exactly so it's interesting that Jenga piece you see think of a piece and what is a piece and the only real functionality in here is that it's responsible for putting itself back yeah and really what you'd want to do is you have that original position you're probably better off having a call called move to position and then in the Jenga tower you'd iterate through all the pieces and you tell each piece to move back and then flag when he's done rap but it's not a bad like it's a fine solution the only problem is that because it's been update what's happening is every single update you're saying you know move back move back I should move back move back now okay I'm moving back and we're back oh we're done occasionally it's just like the update is really happening right a fit to your frame rate whenever something's an update think about the fact that it's doing a lot of work now again I know we're being a bit pedantic because there are should have sample flags but realistically you could have all of this code not in an update on the pieces and then the only person who cares is the actual tower goes should I should I every bill cherry build no okay I will tell everyone to rebuild all of a sudden you've cut out every single piece doing an update the only person used to update is the tower itself yeah so it's it's not a big deal performance wise but it's nice it's logically more sound to say give one person that job and what's what I think is important there is that in it being logically more sound I think it makes more sense to a reader you know I when I think of a Jenga piece I don't think that it's gonna crawl up and float up back into its own position you know I I would have I would envision someone is responsible for gathering up all the pieces and placing them up and and that's where you know in modeling real life I would model a manager or the person who's responsible for putting it all back together at the end of the game yeah usually you want entities to be kind of small enough and in charge of exactly what affects them and things like movement it may seem like that's the job of a single piece but somebody else will pick it up it's not really like it has no momentum a driving force so yeah I don't point out though as we go forward we're not picking on this code it's just a really good example of a three it's the first thing we picked and we're going to it very explicitly but 90% of unity devs I know kind of code like this this isn't like there's nothing horribly wrong with any but we're saying these are just a few tips and tricks you pick up along the way so this we're not we certainly don't mean to be complaining about is perfectly fine code there are just things you can do to improve it I want to thank direct si for allowing us to put him from putting himself out there it's not easy and I don't want to think we're taking on your code here maybe I should give like a like a promo code or like some unity assets everybody who whose code so we don't feel so bad about it maybe if you have some ruffle and give off some SS you know to anybody who you know everyone everyone who puts the code into a raffle and will get that the editor console editor pro or something yeah exactly all right cool right so I guess other than that I mean kind of one thing you could do is well again we're kind of getting into semantics on some of this stuff is all of that code that's running inside of that update function as Charles said you to kind of read through it to parse what it's doing you can just give it a name if it's moved back to starting position make a function called move to starting position put all that code in it and then there's no real illusion as to what it's doing you know extract well I take a whole lot even the even the f else you know because it's like the whole checking if it's null and stuff you just just put the whole lot just so that's nice and very clear in the update you know yeah what's interesting too is we were talking about how there should be something that manages these pieces if you look at the public interface it has one called set assembly so clearly there is some other script that knows about all the pieces and calls set assembly on all of them so it's already kind of like positioned in a way that someone else is gonna make someone else knows when it's time to reposition so all you really need to do is give that particular script some information about the starting positions and then there you go yeah you didn't go one step further and have that tower build with the pieces and therefore pass itself in or you can have a manager class to do it but it's all academic I wouldn't get too bogged down with that kind of thing I mean you can get cute and you can you can define different tower structures if you wanted to yeah well we're gonna call this again sorry I said starting well because it's moving move move to start or something like that move to starting position or something yeah so it's a very small change we're done but if you look at it now you should be relatively clear if you shouldn't assemble just do nothing otherwise Lucas our position you got a live writer if you all don't use writer look at this it knows that we called this function in update so it's telling me hey this is kind of warning you this is called a lot maybe doesn't need to be yeah yes very nice all right so I think for the most part I mean well actually here's something this is a very good one right here so another little known fact if you look at line 65 especially in the case that this function is a co routine so it's been called frequently enough that wait for seconds so the trick is right I don't get into the whole big thing between declaration and sensation but you create a variable and then you use the assign AB you sign something to the variable and you use it the key word to that is new so whenever you see the word new something is being physically nude up it's being created and in this instance there's a new wait four seconds of a certain duration now this is fine because duration is a variable being passed in every time but a lot of people don't realize this but you can cash that wait for seconds object so if you're doing an update loop that's in a while loop or something and it's saying while this wait three seconds to keep waiting three seconds do whatever make a variable and return the variable that way you're only creating the object once for the duration of all of the updates you're doing now it doesn't make much sense of this particular context I was wondering how you could update it no no I think you have to set it once but if this if this was a duration that happened if it's always four seconds don't write new wait four seconds for instead cache it and then use the same way every time yeah that's cool thought about that numerators are the what you call these coroutines yeah if that's a good man actually you'd be surprised how much performance that actually Nets you if you're being very specific there's also a similar one actually it's pretty any off a good one if you scroll up as well to the update function another one it's not important here because they're actually being terpil aidid both values the quaternion and or whatever and rotations of training in hand vector but most people don't know this too there is a transform set position and rotation so if you go on your transform dot set position that position and rotation this does is it literally cut your calls in half so if you're if you've got some code happening in an update where you're doing position and rotational setting this will do both of them in one call and it's again small performance net but it's a very useful and have if you're if you're doing that know funnily enough that used to be an internal function that only unity had access to up until 2017 I was brought to the intention by the developers of inside when they did a talk on performance issues and they mentioned that that's a private function that should have been made public and so I think they announcing it on stage at your knife might have been the moment where Beauty goes okay we'll make that one public yeah that's nice very good but again obviously in this case we're at lighting so I wouldn't do the warden work for what we're doing here yeah so anything else you're spotting there Charles except for what bothers me personally it's the Java naming conventions I'm sorry but you're killing me look at all the method names the casing oh yeah gosh oh yeah how come that didn't its Pascal case there instead of yeah it's not the end of the world again a lot of this stuff is nitpicking but it's a matter of consistency someone else made a joke earlier about the bracketing formats and I don't think he realized he was doing it but Charles was fixing it as he went which is yeah so so there's there's multiple languages of different patterning styles for for brackets and stuff now it doesn't matter which one you use realistically but if you work in a certain language the people who were trained in that language will be more likely to use one of the other if you if you work in Java people are more likely to use pass case method names and they're going to use the is it turn I wish forget the names but it's the bracketing case for last the first practice on the same line and the next one is underneath but in c-sharp people are more likely to use that they're going to use upper case or it's called capital case I always forget but they're gonna basically use capital letter for their method times it's just something to be consistent so right now Pascal is the starting letters lower and then it's capital no that's camel case right sorry I'm complete your honor hands up Charles I got this right oh my god no you gained a lot everyone's calling you a wizard in the chat I'm just like removing these yeah very helpful yes so I call this one start start assembling but it actually shouldn't be start assembling it's like set assembling status I don't know what the heck what is he gonna be called sure because it's true or false I'm gonna do this okay yeah I don't mind starting to stop assembling yeah actually yeah sorry they're gonna do that you're gonna do the opposing one that's a good idea so so the reason Charles picked up on that one is he noticed there was something in there called a flag variable and basically you want to avoid flag variables because what they basically say is this thing does X if it's a Y and Y if it's an X you're basically saying do this if it's true do the other thing if it's false and what happens is you end up with this weird case where you'll have multiple ones of these and you have some method with true true false true or some of that effect and it makes no sense because it doesn't matter the what it does can be completely inverted if you change what the truth and false is do so it's better practice to be very explicit and say do this to start it do this to stop it and then you do the flags internally and if you want to do if you want to pass in a boolean you're better off actually managing that yourself it's a small memory cost but saying if is assembling stop assembling else start as I'm going the power you get back from the readability of that for an entire team is far better than assembling true assembling false yeah we think about this enable physics after timeout I don't know I'm just trying to make this make more sense from from someone who's reading the code and like yeah it's like I didn't it doesn't say anywhere that this is seconds you know you're just like why pass on a number what is that milliseconds is it minutes is it yeah think about all these things I see this every day in my my day job code that I'm just like okay what is it what what's the end what does it stand for yeah yeah that can never be a case of a confusion one day I will say this is a personal thing poor dick attention so people may really hate this something I do myself so this is a you do not take this as gospel under standard rename that function for a second job that's what here yep do a refactor rename on it you know me dude like a straight-up the reason I'm just calling it I'm just typing it oh yeah it's not being called anywhere so well in that case put the capital C in the front of it capital C like that yeah lowercase o o underscore no no just underscore its you know square oh you do it like that now just like that'sthat's how I always do it there's a very particular reason I do this and the reason I do this is you should never call the core team because when you call the core routine what you're saying is this is a piece of code that needs to run in unity inside of a start-over team so what I do is I use Co underscore for the actual coding and then I make a function called whatever the name is so I might have Co underscore and they will physics and then I will have enable physics and what are the naval physics do it calls start curry teen Co underscore enable physics and that way I decouple the fact that it's running a co routine from the fact that it has to be ran as a code in again personal choice but I find it easier to find out which calls can be called directly versus which ones are which ones are meant to be rather coding I prefer and I since I'm driving I'm gonna go ahead do I prefer a pending co-routine at the end of it that's fine this is my I just don't like underscores and a function it's just like it but the points the same right we're both saying whenever it's meant to be ran as a core routine you should somehow indicate that the core routine because people will get confused when they run it and fact you know it's not doing anything oh I forgot and you'll do this yourself you will call a covert gene and go why isn't it working you'll go back and go I forgot to pass it into a start quarter tune so yeah for sure looks like it was being cold and I got lazy and didn't use the actual for factoring yeah so just it's all about being verbose you know like what's my favorite well my favorite quotes from I think Martin Fowler is any any dummy can write code a computer can read but it's it's hard to write code that a human can read something like that yeah you're I mean you're writing code for humans to digest here you know so you just want to make it as clear as possible yeah coatings are very specific to the unity and you would never use this function the way it's written in any other context so yeah it's very so like that I normally don't like using any kind of prefix for things but something like that sort of it's very clearly indicates it has to be used in special case in the interest of making stuff read clearer as well if you happen to be working with doc net framework version 4 or higher you have a couple of nice syntactical tools your disposal to make your code a bit you know cooler looking which charges are demonstrated right here exactly yeah so this what's funny is if you do the automated code cleanup it gets rid of that or does it maybe not I shouldn't I wouldn't myself it did it to me once or a couple times and I was like really I like the a whatever I guess I was wrong or maybe they change it who knows oh no look it did it thank no interesting that's maybe you've got your stylesheet your your style rule set different from mine yeah probably I mean look format selection there's okay this three types strict format compact format and spacious format yeah if you'd be spacious look gosh so long as I don't need it as much as you do but it's that's fine what we're doing we're doing a compromise here yeah so oh look someone asked what do you think of using summaries are you ready that's what I think I mean if I was running like an API that I needed some other tool to automate the creation of the documentation for because a whole bunch of people were gonna use my API and I needed to make sure that they weren't emailing me around the clock saying hey what is this - what is that - sure I'll make us I'll use a summary but only because using that summary will allow me to generate a nice little webpage that you know has documentation only because I don't want to be bothered by people asking me what it does that's yeah that's that's what I feel about summaries so yeah yeah personally I don't like I know it's a cliche thing to say but where do your books go bug trackers don't use to do tasks don't write yourself minds tell yourself what to do in your code where do comments go well your code should be clear enough to read and if it's confusing you should have any documents associated with it to explain what it does at the point in the day code is where your code goes and you can put comments in some people are very you know dogmatic about it and I really I don't care enough it's not evil to put comments in your code it's not over like people who say your code must be self commenting that's it back to be a bit of a meme when people say that but what they really mean is in theory in an ideal world your code is clear enough to understand but it's perfectly allowed in the context of something being confusing to annotate a good example of this is if you're using an algorithm that's not your own if you're calculating some maths or some you're doing something like oh people you're generating fractal patterns you might want to put a comment over the function to do that to explain what it is you're doing now you could call it generate fractals but you might want to put a piece of code in to say you know this uses this particular algorithm things like that might be interesting to kind of help a team understand what you're doing so same with using constants like for example you can create a variable called gravity and you know use that in search or code but if there's certain kinds of constants which might not be well known to people you might want to explain why it's needed and certain algorithms so comments aren't evil you can't use them but if you're commenting to say this set health this thing gets the health or this thing sets the hell it's like why are you writing that if it called set help get health sensibly works you know comments should give you information that lives outside the scope of the code you reading totally agree I don't know what I'm doing areas I went crazy on this refactor I was trying to get rid of all these calls to I was trying to be dry but I think this is kind of overkill you ask me no I I tend to do similar stuff yeah well I think it's sort of like you say uh I misspelled position well it's sort of like you say like you know how the private modifiers and highlight you you know it's like every bit of information you add to a class every bit of code you add to it it just makes it that much more you have to read and internalize and I almost think like this whole generate random position function that it's only job is to provide variable for the random force vector function which is only used in this fling in random direction I almost feel like it just takes away from what this Jenga piece class does yeah that's where I'm like you know if I come and I look at Jenga peace class I don't expect to see generating random position or random force vector like what the hell is that I'm gonna just be like what does this person doing here maybe fluid random you know I mean I'm sorry is position just still stopped as a small side note again we're not actually paid by writer despite what some people might think but it just happens to be very good tool to use and what are the things that could popped into kind of the worth pointing out there is if you scroll up one of the functions above is not being used and I can tell that at a glance cuz it's grayed out so I'm 63 there you can tell it gives you all of these because they're just a it's just a single script that Charles copied in we're really low in this case we can't do what does it add component menu is it the 1i its context many other things what you're thinking of yeah it is no no just gotta go you Scotty yeah so that's a fun one actually in fact we can properly demonstrate that yeah I might as well take a full story so if anyone's ever seen this context when you tip before it's really really handy if you want to test your code you can go and do a whole if impose get button down fire one run my scripts and test it or you can give yourself something to click to actually test a function the the kind of the long-winded way is you use a custom editor and make a button and go through all that jazz but it's a lot of work for what basically boils down to I want to run this code and it's usually not a good idea to have an update call which just does a if button test code because then you're baking in that test into the script when it's not really part of the script instead you want to expose the ability to run that code without having to actually make a test and that context menu or it'll let you do is it'll when you create an instance of the object and you have a script you're actually able to run that function without actually having to create a button for it by just using the drop-down menu on your scripts I'm trying to make a Jenga piece here keep keeping it thematic I like it dare I dare a let me just make it material to make it look oh god what are you doing really scum on that you guys don't want me to deserve this this blank looking block we're programmers you're meant to have developer art it's not meant to look good I'm gonna call my material this is a very good question and I won't told you to answer this first dress okay let's hear it what is your first best practice that goes out the window when you're in a hurry to develop something huh I don't write unit tests that's that does that is definitely the first I think just because it's in fact there's a lot of projects that don't warrant the full unit test practicing I've but on a more granular level on a more practical typing code thing let's talk about refactoring for example what we factors slash conventions you don't do when you're you know I mean you sort of saw me do it here sometimes I don't always use the the shortcuts I'll just like rename a function and everything will break and I'll have to go through piece by piece and actually hand fix all of them misspelled you know method goals but you know that's one of those things where it's like not I guess not everyone is a power user a writer like we are and so yeah that's one of them I I can think of some more what do you got sounds like yeah might well my first one came to my mind when I read that was how I bind my objects and we should cover this as a oh yeah someday okay tuck them in the code they're like really deep into there yeah yeah when I'm when I'm doing it the right way I like to have providers I have separate factories and things like that when I'm in a hurry I will do well what I do is normally I will put the requirements as children to me that and I'll call the game object not find and it is a hierarchy tree walk what is a tree walk under a guaranteed subset of a few small objects so it brings back the thing I said before the component system were ever humanly possible I try to avoid walking the tree if I can pass a reference into an object without having to do any kind of search to find it it's considerably faster and more efficient than doing any kind of stepping through code if I'm in a hurry I will use game object I find and I'll use get component only those two though I won't use find object by type and I won't use find out that in parent those things can lead to some weird scenarios I'm not saying never use them you might need them in certain cases well I'm in a hurry I'll use either of those or I'll use just a public field and I'll drag the reference in so yeah there might there might like quick I'm in a hurry just just just get component everything just grab I don't care I'll forget out later yeah yeah true I think I'm getting a little too Carraway here so I'm one of the comments is the over-engineering started so I think you're setting a bad thing this is Jagger what I see your goldbricks as also it's it's the fancy rich big yeah I messed up my prefab year what the heck it's because they I removed this whatever all right so what's an element ends the rigidbody to right well first of all show the component haven't actually gotten to that bed what Dad component many of the whole thing you were doing oh yeah yeah all right so here you can see it's probably really small - and shoot let me move this over oh there he was - actually the assembly functions already had them did even knows we mean it's our assembling stop something my mistake sorry I thought so we got fleeing in a random direction and start assembling and stop assemble which are the ones that we just added so I'll switch quick to the yeah so quick and dirty context menus will let you add test your code really quickly very handy very useful for throwing things together uh one of the comments there is do you use comma dot main never never use camera domain do not under any circumstances can use it in the awake function to cache you camera so the way I tend to do this just actually make it make a quick trip there Charles and I'll show you sort of how I personally favor using cameras in any stripped pretty much all right so some some test model behavior yeah okay awake what I like to do is yeah at first above that just do a public camera call it camp yeah and then inside of twelve there you go gamma equals camera don't mean no camera equals at camera and then space double do it Knoll do an old propagator count equals camera yeah I have two questions and there we go and then right yeah so it'll it'll it'll ought to change it for you because I think doesn't want you using the null conditional it's the same thing I personally prefer that syntax but it's the same code effectively what you're saying is if someone using my script provides a camera to use I will use that camera if they have not provided me a camera to use assume you're using the main camera so that's my go-to but there is no click instead of serialized field well yeah probably serialized private whatever it's fine I just didn't could it be quick that's not relevant yeah if you're ready really specific fine by that look the under spar camera and our camera double question mark so if you're gonna do the whole exactly how I writers and double quote no yes effective yeah exactly oops so that is literally how I how I would use camera references and I would hide that warning because I don't care about it myself oh no I by the way I never use expression bodies for these event functions I try not to yeah I don't want you yeah cool so there's there's a you can hide the inspection that doesn't matter but yeah so basically the point anyway is the fact that you give someone consuming your code the option to provide an alternative camera otherwise use camera domain but you do it in a wake to cache it because if you actually call camera dot main screen point array and your update you will be calling a find object with tag with every single call oh and it's worth noting I'm that not actually in the hierarchy if you can click the camera mm-hmm am I playing with the server because they're not gonna be able to see it yeah okay so the words camera dot main literally get the camera where that tag is equal to main camera if you had that as untagged and you wrote camera dot main dot whatever it will throw an exception you'll get a null reference because it won't find it that's nitric that I hate yeah it's part of the invisible magic that confuses people so it's not it's not a magic function it doesn't know anything special about your code it's literally just finding the thing with that tag called camera so be very careful using it I only use it in the instance where someone else does not provide me one and even then I tend to check for null but that should give you a good example of how to sort of use cameras in general yeah I'm putting this over here all right so the next so if we're gonna do this it needs to have like a rigidbody right I think right sure that's pretty much the only other thing it really needed yeah where'd your body yet transform alright so uh let's see if we can make this thing do something cool oh and each one of these pieces needs an original position that should be cached on the way across to be creators is it yeah I think you're right okay all right cool perfect so uh do we have everything wait no I didn't update the they're rigid the prefab oh my gosh I'm confused I'm confused by this cuz my defaults messed up what you guys think about my my default setup here I'm curious because I try to model it after using IDs and blender alright cool so let me see here yeah why isn't this updating I thought I updated the prefab variant rigidbody yeah okay and component to the boom so we'll bring that up in the Jack Paar component is a cool script add a fun fact on that one as well just to be very careful with the invisible magic if you put require component on a script while the script is already on an object it won't retro actively require that component for you oops oh yeah I don't know floor I think gravity right I did an actual sword a big rectangle oh yeah uh I'm like screw physics alright who's that that friend of mine Nick oh nice so yeah I'm gonna get a cooler accent like you're clean boom you know I got the planar material now now I'm just getting I'm not going to alrighty so now what I should peel to move it look I'm gonna play Jenga all right so now we can go to one of these components here I think I can I not know just redo do one tonight accrue the point all right so we're gonna do start assembling oh well that's where the the kinematic love to fling in a random direction oh yeah pretty cool job and then uh let's put it back right start assembling nice okay so yeah now can you talk can I walk it and do a more but this actually highlights a very interesting point hey the fun factor I think we found it nice game wait wait before you interesting point that the first the first time you did that there is a small bug which was you picked one piece and tried to get it to return but what was happening was it was bouncing off the other stuff which were because the kinematic things took kinematic so this kind of highlights the point you may conceptually think of each piece as an isolated piece but the task of building a tower requires all pieces to do their job and so you'll inherently have a confusing issue where things will clip in and jitter off each other unless they're all kinematic when the builder goes to happen so that's that looked exactly like a one piece coming back breaks the other piece so that sort of proves the point that something like this should be a should be in a tower object that iterates through make sure everything's kinematic and runs them all because if you do it on a single piece you're gonna get free bugs like this but I'm glad we didn't break it I'm glad the code still works after we after we oh that was never any question in my mind [Laughter] Wow alright that's great feels good I think so let's let's do a little recap here so we did some optimizations right by caching the gate component and the transform people and I always I sometimes forget that the transform is something you should cache we talked about you know obviously some formatting here like just following c-sharp standard net standards of putting the curly braces on their own lines it's a point of contention or conversation or discussion whether or not you want to explicitly put the private modifier I'm for it Jason's against it you and and of course we've got to talk about the fact that we have to use you know which casing camel or Pascal which I know and the most important thing to talk about that is that if you're on a team you should do whatever your team as a whole agrees on because that yeah clearly the most important thing I will notice one thing cropped up during this though that didn't get the covers it's worth mentioning so if you're using the Unity engine random class you might occasionally hit a really weird scenario that might confuse you and so delete that yeah I think that line and we'll just explain a target pure because yeah this will computer do that for me actually yeah so what happens is if you try to use random there's actually two randoms there's a random class which lives in the system namespace part of c-sharp and there's a random which sits in the unity engine namespace so what you can do traditionally you can write you know the engine dot random dot whatever to get it to work but it's worth noting that there are different things yeah and it's you don't need to add them is a system random is someone's static I forget no around so system is the one that isn't static but the unity one is static oh you get the idea yeah but the point the point want to get across here is that it'll usually what you want to do is we're gonna write Unity engine dot random dot range in most cases if you're to avoid issues when you bring in the system interface or what writer will do for you automatically which is another thing worth noting is you can use something what's called namespace aliases and it's where you give something a new name so a good example of this exactly it's perfect so what that does is it says for the duration of this class the thing called unity engine at random will not be known as random and you can do this for anything you like so as a good example you could say you can happen you can call her but oh you want to yeah and it doesn't just have to be run it can be your own types as well you could write say like go on the next line there right using my vector equals vector three or you know the engine defect or whichever because there's different one there you go semicolon so now every time you write my vector it's as if you've written no no this is this specifically it's not a writer thing this is actually a c-sharp thing so it'll work everywhere even in unity van outside unity the only reason we mentioned writers it'll do it automatically for you if it knows that you're doing this with unity little-little do it but you can alias anything you like a good example of this that I use in practical terms of my code is if you're using an API you might have something called a user so you're consuming an API from the web that has a user and in your application you've got something called a user and the one that you have has different properties and different things so what I like to do is say I'm using a Google API and getting their users for sign-on so I will I will alias it to Google user and then I'll have a user in my class and they're both technically called user and the compiler would complain if I tried to use both things with identical names and go to different things so instead I hate is it to something that'll be easier for me to use yep so yeah I noticed that you're using API is that you're constantly you know adding the model classes and you're like a crapshoot what else could I call it other than what it is you know what I'm getting so cool well I think we done did it yeah you got a lot coverage such a small script I have to say I didn't realize the day would be like this is great well so all right those of you who came in through and are watching the vaad feel free to join her to score server my camera just went off feel free to join our discord server and and drop your coat off in a paste bin link in the code review channel so I think I need to think of a better way to do that because that for me they kind of get lost yeah yeah so we need to think of maybe a submission maybe I mean maybe people can post us get up and give that you know publicly making you can actually just add a gist GIS T and it's just a gif of link to your individual files so introducing source code which you should be using source control for your code I should say you you should be able to give us a link directly to to the ones you want so we'll think of a way to get these look I'm a nicer submission process I mean if you email them to Charles an infallible code comm that I'll definitely see it but you know we'll look in the discord server before each stream and see what what was posted in the week prior yeah so one of the comments is do you use namespaces and the answer is yes always on everything it's a hundred percent that's probably a good thing to mention here too so for people who aren't familiar with what namespaces really are they're literally everything you're seeing here using system that collection using unity engine using random these are all namespaces and all the namespaces is think of it like a folder for a single script but the difference is the folders distributed across your files so everything in the same namespace all gets lumped into the same box and when you give your code to other people you're at risk of having collisions with their stuff a good example of this being system dot random and unity agender random like these are two different systems that exist in isolation that both want to have something called a random and so it does it does you service and everyone else who uses your code as service to namespace everything you do and that way people know it's Jason's not random for everything to do with that particular thing as for what you should use for a namespace I usually recommend you name it based on the project or you name it based on what the actual thing is the the concept yeah well I just used luckily it would be it might be infallible code or it might be in this case it would be Jenga system Jenga game yeah that way everywhere think it'd be honest I'm like well this is a game first of all and it's in unity so unity and it's a game here's an interesting thing to note two of you guys are right our users you know this bothers the heck out of me it'll be like hey did you mean project scripts is that what yeah yeah it does more stock projects that's a way you can do that is you can in the actual unity Explorer here on the left you can right-click the folder hit properties and you just uncheck name space provider and then it will no longer it'll no longer suggest that I didn't know that I used to what I want you to do is I just turn off the suggestions for my name space and say I'll do my own name places I don't I don't need you to tell me how to do names faces I do but that's probably a better approach it does cinahl complete that's what I always do I don't know why it's good it says move to global namespace there's good I think there's another way to like actually say hey what what you want the project to automatically namespace to but yeah that's yeah that's what I always do and then the question of namespaces it can be able to get tedious to wrap every single script you write in a namespace and this comes back to what we said earlier make a new template file making a new base template for your entire unity instance and every new monobehaviour you create will have a namespace with your name on it and sort of cleaner functions I will I will actually post the link to the old video i wrote i made and i will call put it in the link in this description of this video i wonder if this will work the i just said the root namespace of this project and in the actual like to go to the you're gonna give it dangerous here i don't like messing with named faceless for that for all projects you're gonna end up getting collisions with learn is to fail miserably i'm gonna call this it doesn't like j it doesn't like Jinga cuz it doesn't it isn't how recognized it as a word look this computer alright now it does a oh there you go we'll have it let me see if i make a new a new script so unity go to scripts I'm gonna say add I know there's a mono behavior here somewhere behavior C sharp script test script hey look I did it okay to explore back to unity oh that's a good point yeah I mean agreement I don't recommend using your actual name for your names places I have done time to time and I usually do when I'm releasing it as educational material or something just so that I have a reference of people know where to go to get more information on it but if you're a people the library for people to consume don't put your name sometimes you have a company name you might use again depends whether it's a product or something that people might want support for but as a general rule name it based on the project it is so for example my technically my company name is called darker smile and I work with a contracting firm for producing projects called fire Panda and so whenever we make work I will prefix it with fire panda DAF Jenga system and then it would be our code inside of this particular project it's not complaining about it we learned something new today yeah until it until it blows up in your face yeah all right well I think this is a good time to wrap up this room what we're doing yeah I think we got through an awful lot more stuff let's so we're gonna try we're gonna try to make these code reviews a weekly thing because they're they're been very this was really fun and educational so I haven't picked day of the week yet to do these streams but eventually I think probably starting in 2020 I'll have a better idea of like hey look it's gonna be every Tuesday at 3 p.m. Eastern Standard so keep an eye out for that in the meantime though if you just follow me on Twitter and fallible code and then if you're on the discord server you will get a ping whenever we we're about to go live so if we didn't answer your question obviously save them keep them in the tank we were probably gonna go with the format of starting the stream with questions and then hopping on over to it like a code review and if we have time we'll go to an either another code review or come back two questions

Channel: Infallible Code

Views: 25,703

Rating: 4.982851 out of 5

Keywords: unity code review, common unity mistakes, unity optimization, unity optimization techniques, unity optimization tips, unity c# classes, unity c# simple game, unity for programmers, unity, unity 3d, unity3d, game developement, gamedev, game development unity, game development unity 3d, unity game development, unity game dev, game programming, unity game programming, programming in unity, game programming in unity

Id: rD8dRtBqTy0

Length: 61min 4sec (3664 seconds)

Published: Tue Dec 10 2019