5 minutes of looking at the client only

[Joost]

something wrong with all of these, or a way to do it better. If code was properly discussed, mistakes could've been found and taken out. a lot of these mistakes are all over the client.

Code:

Public Sub SendGetClasses()
Dim Packet As String

    Packet = CGetClasses & END_CHAR
    Call SendData(Packet)
End Sub

^Problem : Dim string, set string, and using string right away in the packet. Seems 2 steps more than required. And it's like this every packet

Code:

For i = 1 To MAX_TRADES
        With Shop(ShopNum).TradeItem(i)
            Packet = Packet & SEP_CHAR & .GiveItem & SEP_CHAR & .GiveValue & SEP_CHAR & .GetItem & SEP_CHAR & .GetValue
        End With
    Next

Code:

Problem: Most shops don't have all trades filled, sending a lot of empty data
    Select Case Err
    
        Case -2147024770
            Call MsgBox("dx7vb.dll is either not found or is not registered, try re-installing directX or adding the file to your system directory.")
            Call DestroyGame

Problem : If dx7vb is not found, Mirage will crash on startup, this will never be called.

Code:

If CanMoveNow Then
                Call CheckMovement ' Check if player is trying to move
                Call CheckAttack   ' Check to see if player is trying to attack
            End If

Problem: Instead of responding to the keyboard being pressed, we need to check every time if a key's being presed 0.o, that's just stupid.

Code:

If GetPlayerInvItemNum(MyIndex, i) > 0 And GetPlayerInvItemNum(MyIndex, i)  ADMIN_DEVELOPER Then
            Call AddText(Text, Color)
        End If
    End If
    Debug.Print Text
End Sub

Problem : I forgot, sorry.

Code:

Public Function isStringLegal(ByVal sInput As String) As Boolean

Problem : If I recall right, checks letter for letter, instead of checking entire string once

Code:

Private Type NpcRec

Problem : Non-hostile NPC's have 10X more data than required, str/def/hp/exps/itemdrop etc

Code:

Public Sub HandleKeypresses(ByVal KeyAscii As Integer)

Problem : What's more likely? /command or admin message? Should check / before admin, changing order basicly.