5 minutes of looking at the client only

5 minutes of looking at the client only - Printable Version

+- Mirage Source (https://mirage-engine.uk/forums)
+-- Forum: Mirage Source (Nostalgia) (https://mirage-engine.uk/forums/forumdisplay.php?fid=61)
+--- Forum: Archive (2006-2011) (https://mirage-engine.uk/forums/forumdisplay.php?fid=18)
+---- Forum: Source Code Development (https://mirage-engine.uk/forums/forumdisplay.php?fid=51)
+----- Forum: Mirage Source 4 (Visual Basic 6) (https://mirage-engine.uk/forums/forumdisplay.php?fid=44)
+------ Forum: Bugs Reports (https://mirage-engine.uk/forums/forumdisplay.php?fid=9)
+------ Thread: 5 minutes of looking at the client only (/showthread.php?tid=2674)

5 minutes of looking at the client only - Joost - 02-04-2009

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.

Re: 5 minutes of looking at the client only - genusis - 02-04-2009

hmm that's new stuff that DFA added since i stopped helping. You are right about allot of it, but i think the most important thing right now would be fixing the connection errors people gets from the client trying to connect to the server first.

Edit ... why is canattack in a movement If ?

Re: 5 minutes of looking at the client only - Joost - 02-04-2009

This is a bug fix? Some kids asked me to give some input, so this post really is to show some things that're dead wrong.

Re: 5 minutes of looking at the client only - Joost - 02-04-2009

Server-side :

Code:
Public Function PasswordOK(ByVal Name As String, ByVal Password As String) As Boolean

Compares Ucase, while a good p/w is bother upper and undercase.

Code:
CanAttackPlayer

Checks if something has 0 hp, while taht's impossible

Code:
Public Sub DestroyServer()

Does the same as LoadServer, unloading evreything, with a succesful DestroyServer, there's no need to Unload everything again on the next start-up

Code:
Function IsMultiIPOnline(ByVal IP As String) As Boolean

Poor coding.

Code:
Public Sub SendDataToMapBut(ByVal Index As Long, ByVal MapNum As Long, ByVal Data As String

Poor coding.
The entire HIGH_INDEX
Will look something like this : 1, 4, 7, 9, looping from 1 to 9. Instead, an array should be made holding those numbers.

Code:
Public Function CanPlayerBlockHit

Nothing wrong with this function, it's just that shields are completely useless.

Re: 5 minutes of looking at the client only - Jacob - 02-04-2009

So, why don't you try to explain what's wrong with all these?

From what I'm seeing, there's nothing that bad really...

@Genusis: I would guess that the CheckAttack in the CanMoveNow check was a way to stop players from attacking while they were moving.

Re: 5 minutes of looking at the client only - Joost - 02-04-2009

Pick one and I'll explain what's wrong with it. Not going to bother with others considering this project isn't related to the commnity, it's DFA's project, or so Ive been told.

Re: 5 minutes of looking at the client only - genusis - 02-04-2009

well since DFA can't work on it right now since he has no internet at all and hasn't even tried to use the library internet it seems to have turned into a community project.

@dugor i can still attack while i move?

Re: 5 minutes of looking at the client only - Joost - 02-04-2009

If we do this my way, meaning every change will be discussed before added (slow progress) I'll help out.

Re: 5 minutes of looking at the client only - GIAKEN - 02-04-2009

That's how it should be done anyways.

Re: 5 minutes of looking at the client only - genusis - 02-04-2009

when i was helping out DFA i made him test it before we released it but then you complained that i was helping out so i stopped and he then began to just release things without testing it hoping it worked.

But I'm in for testing to me it doesn't matter how long it takes as long as it works.

Re: 5 minutes of looking at the client only - Joost - 02-04-2009

Of course, there're several talented people here. If they are able to comment on each other's code, the project will be stunning.

But of course, another option is to let Genusis do it.

Re: 5 minutes of looking at the client only - GIAKEN - 02-04-2009

genusis Wrote:when i was helping out DFA i made him test it before we released it but then you complained that i was helping out so i stopped and he then began to just release things without testing it hoping it worked.

But I'm in for testing to me it doesn't matter how long it takes as long as it works.

Alright stop bringing that shit up over and over.

The point is, whether somebody checks it or not, no letting shitty code in. Now we all need to check it so this doesn't happen anymore.

Re: 5 minutes of looking at the client only - genusis - 02-04-2009

hey i would not release anything unless i knew it was working =[ making me feel bad.

Re: 5 minutes of looking at the client only - genusis - 02-04-2009

GIAKEN Wrote:
genusis Wrote:when i was helping out DFA i made him test it before we released it but then you complained that i was helping out so i stopped and he then began to just release things without testing it hoping it worked.

But I'm in for testing to me it doesn't matter how long it takes as long as it works.

Alright stop bringing that shit up over and over.

The point is, whether somebody checks it or not, no letting shitty code in. Now we all need to check it so this doesn't happen anymore.

Rodger. we should all check each other code regardless of skill.

Re: 5 minutes of looking at the client only - Joost - 02-04-2009

Added comments to all mistakes I found.

Re: 5 minutes of looking at the client only - Matt - 02-04-2009

Some of these are not useless. Well, one I seen. Lol.

Some of these are just basic security checks. Such as the one checking to make sure you're not higher than the max_items. A good packet hacker could get higher than that, in an attempt to crash the server. Which, he would crash the server, were that check not there.

Re: 5 minutes of looking at the client only - genusis - 02-04-2009

Quote:Problem : I forgot, sorry.

Code: Select all
Public Function isStringLegal(ByVal sInput As String) As Boolean

the issue is it doesn't properly calculate the string data and is inaccurate.

Re: 5 minutes of looking at the client only - doomJoost1 - 05-04-2009

Matt Wrote:Some of these are not useless. Well, one I seen. Lol.

Some of these are just basic security checks. Such as the one checking to make sure you're not higher than the max_items. A good packet hacker could get higher than that, in an attempt to crash the server. Which, he would crash the server, were that check not there.

The check is client-side, making it useless. But this is exactly what I want, snippets of code that can be discussed by anyone before it gets added.

Re: 5 minutes of looking at the client only - Matt - 05-04-2009

doomJoost1 Wrote:
Matt Wrote:Some of these are not useless. Well, one I seen. Lol.

Some of these are just basic security checks. Such as the one checking to make sure you're not higher than the max_items. A good packet hacker could get higher than that, in an attempt to crash the server. Which, he would crash the server, were that check not there.
The check is client-side, making it useless. But this is exactly what I want, snippets of code that can be discussed by anyone before it gets added.

Yeah, you pointed that out on msn. Tongue