Jump to content
  • Sky
  • Blueberry
  • Slate
  • Blackcurrant
  • Watermelon
  • Strawberry
  • Orange
  • Banana
  • Apple
  • Emerald
  • Chocolate
  • Charcoal
Bixby Sayz

And this is why comments are a good idea

Recommended Posts

Don't really have a rants section, so I'll stuff it in here.

 

Trying to figure out why OSI's RSReady function returns False all the the time and I see this with absolutely no explanation:

[sCAR]if (not SimilarColors(GetColor(381, 56), 32747, 5)) then[/sCAR]

 

That coordinate is in the middle of the screen. Even on the old client (looked at some screenshots) it's in the middle of the screen. Since there are absolutely no comments on what they where trying to check for we are left to test our physic abilities and guess.

 

This is pet peeve of mine: lack of comments (especially in includes) detailing exactly what you are doing and why. Who cares if it clutters up the code and makes it longer if it means people can actually understand it.

Share this post


Link to post
Share on other sites
Don't really have a rants section, so I'll stuff it in here.

 

Trying to figure out why OSI's RSReady function returns False all the the time and I see this with absolutely no explanation:

[sCAR]if (not SimilarColors(GetColor(381, 56), 32747, 5)) then[/sCAR]

 

That coordinate is in the middle of the screen. Even on the old client (looked at some screenshots) it's in the middle of the screen. Since there are absolutely no comments on what they where trying to check for we are left to test our physic abilities and guess.

 

This is pet peeve of mine: lack of comments (especially in includes) detailing exactly what you are doing and why. Who cares if it clutters up the code and makes it longer if it means people can actually understand it.

 

I would assume its looking for the login text color judging by how the color debugged. Not everyone can be like you bixby your the man when it comes to documentation!

 

[scar]

program New;

 

Var

BMP: TSCARBitmap;

begin

BMP := TSCARBitmap.Create('');

BMP.SetSize(100, 100);

BMP.ReplaceColor(clBlack, 32747);

DebugBitmap(BMP);

BMP.Free;

end.

[/scar]

Share this post


Link to post
Share on other sites

That's what I assumed until I read further. It basically reads: if color not found then check for lobby screen, if not lobby screen then check for login screen, if not login screen then RS is not ready.

 

So it's checking for some orangey color in the middle of the screen while logged in???

 

Edit: Nice way to check the color. I just fired up the ACA tool and manually entered the color.

Share this post


Link to post
Share on other sites

That's my bad, still have to fix that... That was supposed to check if the client was still loading, but it fails...

Share this post


Link to post
Share on other sites

Ah...

 

I just changed that first line to if not LoggedIn so it handles logged in, login screen and lobby screen.

Edited by Bixby Sayz

Share this post


Link to post
Share on other sites

I tried explaining my thoughts on this and I almost shot myself in the head, but instead I deleted it all and posted this message.

Share this post


Link to post
Share on other sites
Ah...

 

I just changed that first line to if not LoggedIn so it handles logged in, login screen and lobby screen.

 

That's what I did originally, but I changed it for some reason, I don't remember why.

 

EDIT: LoggedIn won't actually help in waiting until RS has loaded, because you'll never log in before RSReady is complete + it fails because of white text on the loading screen. I'll figure out a fix later today.

Edited by Freddy

Share this post


Link to post
Share on other sites
EDIT: LoggedIn won't actually help in waiting until RS has loaded, because you'll never log in before RSReady is complete + it fails because of white text on the loading screen. I'll figure out a fix later today.
Not true.

 

If SMART is already loaded and you are already logged in it will reattach to the existing SMART but it will take forever to start while it sits there waiting for RS to get ready. This is what I was looking into: restarting a script takes forever to initialize. Makes debugging/designing a script a real pain. Never noticed the white text/loading screen thing.

 

OSI needs some tweaking and it's not fair that you seem to have been saddled with it Freddy. Considered (very briefly) forking OSI and reworking to not use the + 50 pixel hack. Would be a cleaner approach: Adjust the Globals.scar consts, add consts for the toolbar area, and fix up any hard wired coords. Then I realized with the spare time I currently have I should finish sometime around 2013.

Share this post


Link to post
Share on other sites

The "50 pixels" hack is a solid approach, that's what the new system was designed for, but it shouldn't be used as such on the loading / login screen. But I don't have the time to implement it like that myself + the last time I actually wrote anything for RS in SCAR was years ago, so the dos and don'ts tend to illude me

Share this post


Link to post
Share on other sites
That's my bad, still have to fix that... That was supposed to check if the client was still loading, but it fails...

I believe the original thinking was: If login screen, or lobby screen, or already logged in, then RS is ready. Unfortunately LobbyScreen returns a false positive before RS finishes loading.

Share this post


Link to post
Share on other sites
I believe the original thinking was: If login screen, or lobby screen, or already logged in, then RS is ready. Unfortunately LobbyScreen returns a false positive before RS finishes loading.

 

Well, that was what that GetColor was supposed to do, obviously it failed :) I've written a fix for CurrentLobbyTab, that should prevent RSReady from returning too soon. I've tested it on both SMART and the regular RS client, seems to work.

Share this post


Link to post
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.


×
×
  • Create New...