Shoutbox

Update: I found a bug. For some reason, the saving of settings is saving to the wrong place. Instead of trying to fix the problem, I'll just move over to using the registry for saving settings.

Remember to save settings per user, and not globally! There's nothing so annoying for a user not to be able to set specific options only for that specific account of his. eg: If you don't do that and if your little brother signs in he will also be using the very same settings and can change them to whatever he likes. There is nothing so annoying to reset all the settings each time because your bro messed them up (or have set them in the way you don't like).

The user's email is giving in the Initialize() function, so that can be used to create a subkey to save the settings for that user only.

(same story goes for when you save stuff in a INI file instead)


------------------------------------------------------------

Looking at your code, the bugs are not because of the "not to do for a plugin" list, but because your own code is buggy:

quote:

---

Originally posted by Heffy
Instead of trying to fix the problem, I'll just move over to using the registry for saving settings.

---

code:

---

Public Function InitVars()
    (...)
    ini = regQuery_A_Key(HKEY_LOCAL_MACHINE, "SOFTWARE\Patchou\MsgPlus2", "PluginDir")
    ini = iniSettings + "\Freedom.ini"
    sWry = GetFromINI("Settings", "Wry", "False", ini)
End Function

---

You've nowhere defined the variable iniSettings. Because of that it will always be empty. And thus your ini file will be created/read from the current hard drive in the main root; "\Freedom.ini" (eg: if C is the current hard drive being used then "c:\Freedom.ini" will be read).

To avoid such errors (using non declared variables) always include the line "Option Explicit" as the very first line in every module, form, class, etc...

Also you've defined the global variable sWry as a boolean! So if you assign something to the variable it should be a boolean and not a string. Otherwise you will produce errors!

To fix the bug:

code:

---

Public Function InitVars()
    (...)
    ini = regQuery_A_Key(HKEY_LOCAL_MACHINE, "SOFTWARE\Patchou\MsgPlus2", "PluginDir")
    ini = ini + "\Freedom.ini"
    sWry = GetFromINI("Settings", "Wry", "False", ini) = "True"
End Function

---

Though it doesn't check if the registry key exists or not. So you need to add that too and/or define a default directory. Otherwise, in that situation, the variable 'ini' will still be "\Freedom.ini" in the end and you still would get problems.


------------------------------------------------------------

code:

---

Private Sub cmdDone_Click()
    frmConfig.Hide
    Unload frmConfig
    spam = GetFromINI("Settings", "Wry", "False", ini)
    If spam = "True" Then
        chkWry.Value = 1
    Else
        chkWry.Value = 0
    End If
End Sub

---

That routine is useless.

You nowhere use the global variable 'spam' for anything, so why reading it from the INI file?

Besides the routine being useless, the first line 'frmConfig.Hide' is also useless as you unload the form right after.

Besides that, the unload line is again counteracted by the use of 'chkWry.Value'. Whenever you use a reference to a control on a form which is not yet loaded or already unloaded, the form will be reloaded again (which is rather very logical because otherwise the reference would be invalid)! In other words, if you want to unload a form, do it as the very last thing in the procedure.

Besides that the IF THEN ELSE can be written in 1 line very in a very short way.

So besides that setting the chkWry checkbox is useless because you unload the form (and thus will loose such settings anyways), the routine can/should be written like:

code:

---

Private Sub cmdDone_Click()
    spam = GetFromINI("Settings", "Wry", "False", ini)
    chkWry.Value = abs(spam = "True")
    Unload frmConfig
End Sub

---

And because you wont do anything with the 'spam' variable anywhere else in your project, it can/should be shortenend to:

code:

---

Private Sub cmdDone_Click()
    Unload frmConfig
End Sub

---

------------------------------------------------------------

code:

---

If IsNumeric(sCommandArg) Then
    sRoll = Int(Rnd * sCommandArg) + 1
    sSides = sCommandArg
Else

---

sCommandArg is defined as a string! Therefor 'Rnd * sCommandArg' will produce an error.

code:

---

Else
    sRoll = Int(Rnd * 6) + 1
    sSides = 6
End If

---

sSides is defined as a string! Therefor 'sSides = 6' will produce an error.

------------------------------------------------------------

code:

---

spam = regQuery_A_Key(HKEY_LOCAL_MACHINE, "SOFTWARE\Patchou\MsgPlus2", "PluginDir")

---

Instead of using this API registry function (with all the problems it can cause) to get the plugin directory (if it exists at all!!!!), simply use 'App.Path' to get the directory where the plugin is located (this is afterall the plugin direcotry):

code:

---

spam = App.Path

---

If you do this you wont need to rely on (potential buggy) registry reading and you wont have problems when the regisrty can't be accessed, etc etc.

------------------------------------------------------------


etc...

Lots of other comments and stuff are added in the fixed files which I'll attach later on...

PS: If you do use the registry to save settings, do not forget to remove all the settings if you uninstall the plugin. If you use an ini file for settings don't forget to remove it also when uninstalling (same with the wav sound).

EDIT: Attached is the fixed source, compare each procedure in every module with your original code to see where the bugs were.

quote:

---

Originally posted by Heffy
Update: I found a bug. For some reason, the saving of settings is saving to the wrong place. Instead of trying to fix the problem, I'll just move over to using the registry for saving settings.

---

I think its not that settings are saved in the wrong place, but you access the settings wrongly. There is a line that says ini = iniSettings + "\Freedom.ini", but i think u meant ini = ini + "\Freedom.ini"


Edit: I posted this at around the same time as Cookie

Thanks for the help so far, guys!

However, Cookie, the spam variable is used alot in the project - it's just my temporary variable. I couldn't think of a more suitable name at the time, so I just called it spam.

Edit: On the undefined variable, iniSettings - that's what I called the ini variable, until I shortened it.

quote:

---

Originally posted by CookieRevised
(same story goes for when you save stuff in a INI file instead)

---

Don't forget Cookie , Microsoft is advising people not to use ini's and use XML files to store data in. It's alot easier to manipulate XML files and its also alot better for importing settings or importing data into other applications .

quote:

---

Originally posted by Heffy
Thanks for the help so far, guys!

However, Cookie, the spam variable is used alot in the project - it's just my temporary variable.

---

In other words it is not used thruout the project. A temporary variable shouldn't be made globally like you did. If you need a temporary variable you declare it in the procedure you need it.

Global variables should only be used if you need to variable to retain its value thruout the project.

It is bad practice to simply declare all variables as global (and prone to errors, as you experienced).

quote:

---

Originally posted by Heffy
Edit: On the undefined variable, iniSettings - that's what I called the ini variable, until I shortened it.

---

Use 'Option Explicit' in all modules, forms, classes, whatever, to avoid such mistakes.

quote:

---

Originally posted by Chestah
Don't forget Cookie , Microsoft is advising people not to use ini's and use XML files to store data in. It's alot easier to manipulate XML files and its also alot better for importing settings or importing data into other applications .

---

There is no reason why you should not use INI's, at all. The advise from Microsoft has got nothing to do with security issues, easyness or whatever else. Instead, it has everything to do with the policy that Microsoft takes into trying to push people to use a new way in storing stuff instead of using an old, but very decent and solid way of storing stuff (just as they try to push people from leaving VB and jumping to VB.NET). There is no valid basis why you should not use INI's in any way, especially in projects like this. It is a very wide spread and seriously misconception that using INI's would be bad, less efficient or whatever.

quote:

---

Originally posted by CookieRevised
There is no reason why you should not use INI's, at all. The advise from Microsoft has got nothing to do with security issues, easyness or whatever else. Instead, it has everything to do with the policy that Microsoft takes into trying to push people to use a new way in storing stuff instead of using an old, but very decent and solid way of storing stuff (just as they try to push people from leaving VB and jumping to VB.NET). There is no valid basis why you should not use INI's in any way, especially in projects like this. It is a very wide spread and seriously misconception that using INI's would be bad, less efficient or whatever.

---

yeah, i agree

To be honest, i just write all my settings to a random access text file and then just retrieve them when i need them. Probably a dodgy method, but its fast, reliable and works.