ID:151560
 
Hi!

Well, what I really want to do is get better at programming. But I do not want to release all of my hard work to get taken because then people would never want to play what I create when they could edit it themselves and host it. So, I was wondering if a skilled DM programmer could take 10-20 minutes of their time to run through my source over TeamViewer which is an application that allows you to view the other person's computer with no risk to your own. If you are very well known around Byond I may feel safe giving out some of the files. I really want to break out of any bad habbits I might have that could be holding me back.

Thank-you!
I don't think people taking your stuff will be a problem. Just post some sample code that accomplishes more or less the same thing (showing off bad habits or what not).
In response to EGUY
Ok. Its not that great anyways I guess. Any help is appreciated!

Edit: Ok, it looks like nobody else wants to help, so I might as well remove the below code.
In response to Darkjohn66
Alright, I'll look over the Login().


For line 1
mob/Login()

Fine here.


For line 2
worldplayercount++

This isn't very static. A while back, when designing weapon systems, I learned that changing a variable like this will lead to problems down the road.

You're better off with something more dynamic like adding the mob/player to a list.
var/list/worldplayerlist=list()

mob/Login()
worldplayerlist += src

Later on, you can periodically check the existence of all the mobs in that list. And, you can use it for a Who() proc! Upon logout, call:
worldplayerlist -= src



For line 3
icon = 'mob.dmi'
icon_state = "walk"

You could use a mob specific to the player and define all that there.
world/mob = /mob/player
mob/player
icon = 'mob.dmi'
icon_state="walk"

world/mob is the type of mob the client will get when they connect.


For line 9
src.stunned=1

This looks fine, but just make sure you call src.stunned = 0 so they can move.


For line 10, the return ..() is going to end the proc before the stuff after it is completed.

mob/Login()
src<<"A"
return ..()
src<<"B"
//This returns 'A' only.

Instead, you can use ..() at the beginning or end of the proc. This will cause it to call the Login() from the mob that it inherits from.

mob
Login()
world<<"A"

mob/Tester1/Login()
Login()
world<<"B"
..()

mob/Tester2/Login()
Login()
..()
world<<"B"

Tester 1 will give 'B' and 'A'. Tester 2 will give 'A' and 'B'.

Login() does not even return a value so no 'return ..()' is needed. When defining Enter() for turfs and such, you'll need to use that.


For lines 10-11
var/savefile/F = new("Saves/[src.key].sav")
Read(F)

This should probably be delegated to a mob/proc/Save(). This is your login proc. Make it do login things. Delegate anything else to another proc. It looks neater, makes it more organized, and you can call Save() whenever you want.

You might want to call this after you set newplayer.


For lines 14 and 17
view() << output("Yada")

I'm not sure what your interface looks like, but view(src)<<"Yada" makes more sense here and you've used it in line 8.



Not much bad here. Of course, Garthor or someone is going to come through here and beat the both of us around.