In game clock [Reveiw]

[bretseattle]

This is my attempt at an in game clock. It is made from a gutted gold window and some bits of a playtime window. It isn't a very powerful script because it needs events to do the time processing. Hopefully I am not wasting anyones time by posting such primative script. Any help in streamling this would be great.

Spoiler

#==============================================================================
# ** Window_Clock
#------------------------------------------------------------------------------
# This window displays the time and date.
#==============================================================================

class Window_Clock < Window_Base
#--------------------------------------------------------------------------
# * Object Initialization
#--------------------------------------------------------------------------
def initialize
super(100, 0, 160, 128)
self.opacity = (160)
self.contents = Bitmap.new(width - 32, height - 32)
self.windowskin = RPG::Cache.windowskin"gear"
end
#--------------------------------------------------------------------------
# * Refresh
#--------------------------------------------------------------------------
def refresh
e = $game_variables[32]
i = $game_variables[40]
self.contents.font.size = 35
self.contents.clear
self.contents.draw_text(0, 0, 120, 32, "Week", 0)
self.contents.draw_text(0, 0, 120, 32, $game_variables[34].to_s, 2)
self.contents.draw_text(0, 32, 120, 32, $Week.to_s, 1)
hour = $game_variables[42]
min = $game_variables[39]
text = sprintf("%2d:%02d", hour, min)
self.contents.draw_text(0, 64, 120, 32, text, 0)
self.contents.draw_text(0, 64, 120, 32, $M[e].to_s, 2)
end

def update
refresh
end
end

class Array
$Week = ["Monday","Tuesday","Wednesday","Thursday","Friday","Saturday","Sunday"]
$M = ["AM","PM"]
end

 

[BlueScope]

First, use code-tags next time! If you want someone's help, breaking their eyes while trying to read that bunch of characters isn't too helpful :p
Also, I'm gonna be nickpicky about everything since you asked for a review here... it's not a too bad script, but definately improveable... well, let's get to it:
 
 
Your scene setup is improveable: You have '# * Refresh' above the refresh definition, but nothing for update. If you wanna add those to your scripts, or like me think writing Refresh above refresh is kind of redundant, is up to you - still, on first sight, Window_Clock seems to end after the refresh definition, and update seems to be a seperate script just like Array class.
 
Okay, going from less bad to more bad: Variable names and tasks. You have 'e' and 'i'... and they essentially do nothing but being single instance placeholders for a $game_variables item. Same applies to 'hour' and 'min', though these at least have recognizeable names.
You know that instead of

i = $game_variables[40]

$Week[i].to_s

you can use

$Week[$game_variables[40]].to_s

This saves you four lines of code in your script, since you only use 'i', 'e', 'hour' and 'min' once either way.
 
Your self.contents.clear command is after your fontsize change, effectively erasing what you just did. Setting that fontsize is a waste of processing power unless you put it behind the clear command.
 
Now to the serious (kill-worthy) stuff: Your integration of Array class. While you got it right that Array defines arrays, it's totally not meant to have variables defined in it. Those should always be defined where they needed - now this location can have several ranges: Variables might be needed in multiple classes, sometimes only within one class, and in your case: Only in the active method. You used - because you had the variables in the wrong class - a global variable for defining those, while you wouldn't need anything more than an instance variable (the one without $ or @ before the name) in fact.
And just for perfection: Capital beginning letters are for constants, not variables. While they do not do any harm but process a few bytes more here and there, you should definately go for perfection as far as coding style goes.
 
Also, I think your

self.windowskin = RPG::Cache.windowskin"gear"

should be

self.windowskin = RPG::Cache.windowskin("gear")

As a side note here, single quotes save processing time compared to double quotes. You can't always use single quotes, but where you can, do so! (Applies to your arrays too!)
 
As a special refinement, I'd not use a seperate variable for AM/PM. Instead, have a standart 24 hour clock "running" in your game, and get the value from that: bigger than 12 -> draw 'PM'. It's as simple as that, and you save some hassling with your variables ingame, since I guess you plan to set them manually.
 
 
 
Alright, after saying all that, you should look over your script again, beat the mistakes with a stick and try to improve it. Then, and only then, you should look into the following spoiler, which includes a version of the script how I would've done it (with the commented headers though, because Seph beats the shit out of me if I teach people not to document their scripts :p )
Make sure you call back if there's anything unclear.
 
Keep up the good work!
 
 

Spoiler: BlueScope's Version

So you're sure you aren't just clicking this after you read my post so you don't have to do anything yourself? Alright then, check it out:

#==============================================================================

# Window_Clock

#------------------------------------------------------------------------------

# Displays ingame time and date

#==============================================================================

 

class Window_Clock < Window_Base

    #--------------------------------------------------------------------------

    # Object Initialization

    #--------------------------------------------------------------------------

    def initialize

        super(100, 0, 160, 128)

        self.opacity = 160

        self.contents = Bitmap.new(width-32, height-32)

        self.windowskin = RPG::Cache.windowskin('gear')

    end

    #--------------------------------------------------------------------------

    # Refresh

    #--------------------------------------------------------------------------

    def refresh

        day_names = ['Monday', 'Tuesday', 'Wednesday', 'Thursday', 'Friday',

                     'Saturday', 'Sunday']

        self.contents.clear

        self.contents.font.size = 35 # this should rather be defined globally, so 

                                     # you have the same font sizes everywhere 

                                     # guaranteed. I'll leave it here either way, 

                                     # since there might of course be a specific 

                                     # reason why you especially defined it in here.

        self.contents.draw_text(0, 0, 120, 32, "Week", 0)

        self.contents.draw_text(0, 0, 120, 32, $game_variables[34].to_s, 2)

        self.contents.draw_text(0, 32, 120, 32, day_names[$game_variables[40]].to_s, 1)

        time = $game_variables[42] > 12 ? [$game_variables[42] - 12, ' PM'] : [$game_variables[42], ' AM']

        text = sprintf("%2d:%02d", time[0], $game_variables[39])

        self.contents.draw_text(0, 64, 120, 32, text + time[1], 2)

    end

    #--------------------------------------------------------------------------

    # Update

    #--------------------------------------------------------------------------

    def update

        refresh

    end

    #--------------------------------------------------------------------------

end

 

[bretseattle]

Thanks BlueScope, I reworked my script (below) before looking at yours. I decided to use yours because it was shorter and had some format that I could learn from if I got familiar with it. I will give you credit for your should I ever publish a game with the clock.

Spoiler

#==============================================================================
# ** Window_Clock
#------------------------------------------------------------------------------
# This window displays the time and date.
#==============================================================================

class Window_Clock < Window_Base
#--------------------------------------------------------------------------
# * Object Initialization
#--------------------------------------------------------------------------
def initialize
super(100, 0, 160, 128)
self.opacity = (160)
self.contents = Bitmap.new(width - 32, height - 32)
self.windowskin = RPG::Cache.windowskin("gear")
end
#--------------------------------------------------------------------------
# * Refresh
#--------------------------------------------------------------------------
def refresh
self.contents.clear
self.contents.font.size = 35
week = ["Monday","Tuesday","Wednesday","Thursday","Friday","Saturday","Sunday"]
self.contents.draw_text(0, 0, 120, 32, "Week", 0)
self.contents.draw_text(0, 0, 120, 32, $game_variables[34].to_s, 2)
self.contents.draw_text(0, 32, 120, 32, $Week[$game_variables[40]].to_s, 1)
hour = $game_variables[42]
min = $game_variables[39]
text = sprintf("%2d:%02d", hour, min)
self.contents.draw_text(0, 64, 120, 32, text, 0)
if $game_variables[38] >= 11
self.contents.draw_text(0, 64, 120, 32, "PM", 2)
else
self.contents.draw_text(0, 64, 120, 32, "AM", 2)
end
end
#--------------------------------------------------------------------------
# *update
#--------------------------------------------------------------------------
def update
refresh
end

end

I was uncertain about if all the code tag the script needed was an update header or if I need more code tag to make this more paletable. Thanks again for your time.

 

[BlueScope]

Well, for once, that looks much better and will make your RGSS player sweat less

But you still got some things in there that'll keep this from working - for example, you never substract 12 from your hour variable, which you need in order to display the AM/PM correctly... now, you'll get values like 1AM, 4AM, 10AM, 11PM, 14PM, 20PM, 23PM - doesn't make sense.
And yeah, why i bolded that P there: You'll get PM values from 11 on now, instead of 13. If you want to replace the '>' by '>=', use this line instead:

if $game_variables[38] >= 13

Also, you still got your '$Week' variable in there, instead of 'week', which makes me assume you didn't tested your script at all after reworking it, because this wouldn't display a weekday at all - it'd give you an undefined variable error for '$Week'.

Other than that, looks functional to me... I dunno why you pumped your refresh code so much, you can see my version is way slimmer and therefore easier to execute (since you call it every frame, it has to execute all those each frame, and especially the draw_text method is heavy on the interpreter, so you might want to go with my version instead...)

Also, you don't have to give me any credit. This is supposed to help ya, not to advertise my name around And after all, it's your idea here, I'm just trying to help you realize it.

Side note: Code tags are these: [code*]This is your code[/code*] - without the *
So, a text enclosed in those tags looks like this:

This is your code