Wisling / tibiaauto

Tibia Auto is made to excel in the automation of various aspects of the playing the MMORPG Tibia
31 stars 17 forks source link

Fixed tasender.printText #9

Closed denisdifazio closed 8 years ago

denisdifazio commented 8 years ago

Hi guys, Noen here. (I'm spending more time trying to do a pull request than I actually spent fixing the problem lol)

Fixed tasender.printText function: -Fixed asm parameters; -Fixed removing/updating a string.

dmarszk commented 8 years ago

Indeed git workflow is something that requires getting used to. ;) Looks good to me, didn't test yet.

//edit: Since inject dll is C++ and CRT-dependant already anyway. Do you think you could rework it for more dynamic operation by the way you're messing with HUD? HUDisplay array could be some STL container for sure (vector/list?), so there's no need to iterate over 100 mostly-empty entries each time. This structure now is eating over 100kbytes of BSS section, and it's hurting. :(

denisdifazio commented 8 years ago

Done! Check it out to see if it is good enough.

Wisling commented 8 years ago

Hey Noen! The changes look nice and tidy. I'll test them out and see how they work. Question though. Line 2684: else if (!HUDisplay[loop].pos.x && !HUDisplay[loop].pos.y && found < 0)

Do you know what the found variable did and why it's not needed anymore? Also, could you do a size check/cutoff on the buffer you're copying into vecHUD[i].message ? And maybe declare a constant for the maxsize of vecHUD[i].message.

denisdifazio commented 8 years ago

As the HUDisplay array was static, the loop always covered from 0 to 100. So to add strings to the empties slots of the array,the 'found' variable was necessary. The logic behind it was: if pos.x and pos.y are not found or equal zero and 'found' < 0, add the string message to current loop position. I will change message array size from 1024 to 128, alright?

dmarszk commented 8 years ago

128 sounds fair enough. I'll make it global constant visible from both TA and DLL code during further TAinject code tidying up.

denisdifazio commented 8 years ago

Commited the change in message length

dmarszk commented 8 years ago

Can you check that length aswell, and cut the input HUD string to avoid memory corruption in case someone tries to make it longer? Or just make it a c++ string, actually. Lil slower - much more flexible.

denisdifazio commented 8 years ago

Commited. I changed both messages variables types from char array to string. Rebel how can you check how much of memory an array is eating while in debug mode?

Edit:I also added a contant to indicate max message length. Another thing I observed while testing is that the message sent by the pipe has a char array called "payload" which size is 1024. If a user send a message big enough, TA will crash because the message will exceed "payload" length. Maybe create an dynamic char container as I did with printText, (vector<char> payload)?

dmarszk commented 8 years ago

You need not to copy string buffer. Just use .c_str() member of the string to access its null terminates C string buffer. Checking memory usage of an application is possible through Task Manager. Please note that Debug configuration might eat few times more memory than a Release build. VS profiling might have some better tools for memory usage monitoring. But I never needed to try it this way.

denisdifazio commented 8 years ago

Done! I didn't know about that method. Something else to refine?

dmarszk commented 8 years ago

Looks fine so far. Regarding ipc messages - this requires more work and some deeper thought. You cannot send dynamic containers through pipe. They need to be serialized first.

Wisling commented 8 years ago

Changes look good. You can send messages of any size through the message pipe, but no real need for that here. The size of the message needs to be managed when it is sent, which it seems like it's not. I'll add a text limit on the sending end too.