General > Developers Corner

Command for ulx bring all

<< < (2/2)

Stickly Man!:
Nice work, Timmy! ;D

Megiddo:
Timmy's new function is now built into ULX (GitHub only until release). :)

Timmy:
Yay, awesome!

@Megiddo: It's really cool to see the changes you made. I particularly like how you generate the spiral grid... so much more efficient. I can tell that I still have a lot of room for improvement. Do you perhaps have any feedback? Was my code hard to understand? :)

Megiddo:
Sure, happy to provide feedback.

The main changes I made are as follows (in order of subjective importance):
[*] As you noted, the spiral grid function. Your method worked great, but as the grid grew larger required a lot of inner-loop skips that weren't necessary. Additionally, since such a grid only needs to be generated once, I did so at load time (rather than each time someone is brought) and cached it. Also, if only one person was being brought, I wanted the algorithm to try a center position first.
[*] Along the lines of the above, by removing a more complicated algorithm from inside the command function, it's a lot easier to follow the flow of the command function. Even if I had kept your algorithm, I would have made that part its own function for clarity.
[*] I wasn't sure why you chose TraceHull instead of TraceEntity (more efficient? more familiar?), but I went with TraceEntity for consistency with the other teleport functions and since it's worked well for us up to this point.
[*] Instead of calling player.GetAll() a bunch of times inside the function, I built the list I needed only once (the list was also a little different from yours).
[*] You had an unnecessary Normalize() function in there.[/list]

Some of these are merely preferences, but I always prefer shorter, clearer code where possible. I add efficiencies where it does not overly damage the first two objectives (unless it's in a critical section of code, like a hook library). An excellent book along these lines is Clean Code by Robert Martin.

Your code was not terribly difficult to understand, especially given that it wasn't that many lines of code and I understood the overall trajectory of what you were trying to accomplish. I would suggest comments in the more "sticky" spots, though -- both for yourself and others. For example, it took me a while to work out how I was going to accomplish the grid function, and I added comments which explain what's being generated where.

Timmy:
I really appreciate the feedback. These are things I will keep in mind going forward. I've seen 'Clean Code' in my school library, will definitely check out the book. Thank you!

Navigation

[0] Message Index

[*] Previous page