QUOTE(Ilirith @ Jul 25 2012, 12:05)
Indeed they are, its becoming a tad troubling (IMG:[
invalid]
style_emoticons/default/tongue.gif)
I reverted the skillbutton function to pre-5.3, works fine now (IMG:[
invalid]
style_emoticons/default/smile.gif)
HVSTAT_5.3.3.3.zip ( 170.45k )
Number of downloads: 54I am looking at the code and you appear to have just reverted to an older version as is. There are huge massive undoing of changes including the undoing of some changes unrelated to the bug in question.
For example, the gem display function is now using the version where it encodes it in base64 rather then pointing at the png files... yet those files still exist.
Guys, if we are co-developing we need to know what is going on without spending a lot of time in a diff program analyzing code. Please explain a little about the code changes you are making.
So 5.3.3.3 was a huge revert to the 5.2 branch to fix a bug. I think rather then doing such a revert, it would have been more appropriate to say "5.3 branch is a dev version and until it is done use 5.2 branch to avoid those bugs" (of course then they would encounter other bugs...) and actually solve the bug in the 5.3 branch... unless we are outright tossing 5.3 changes and selecting merging individual parts of it into 5.2... which if we do should be clarified and called something like 5.4 (and I don't think we should do that).
5.3.3.2 was adding conditionals that will drop a function if used out of chrome... yet not replacing it with something that works in firefox nor adding the header needed for it to function in FF... so it doesn't actually work in FF and the code it adds would need to be removed and replaced in a hypothetical FF version anyways. Might as well have an actual FF version without that "is chrome" function and just with code properly designed for FF. What with all the changes needed between chrome and FF... so if you are doing work on an FF version it should be as a seperate FF only file rather then trying to make a single file that works on both. And it should not increment the chrome version but the FF version...
How about using the HVSTAT FF #.#.#.# and HVSTAT GC #.#.#.# in the future?
with first number being major milestones (a collection of features). Second being new features. Thirds being bug fixes or performance enhancing rewrites. And fourth being syntax changes or dev builds working towards a bug fix but not there yet?
Feedback on it would be welcome. and don't be afraid to let a number go over 10. having 5.13.2.31 is just fine. Chrome reads it as five.thirteen.two.thirty-one. So 5.10.0 is considered newer then 5.9.9
Actually, anyone knows if FF reads it the same way? Regardless, FF allows downgrades even if it would get confused on which is newer... while chrome does not (requiring manual uninstall and then installing the lower version)
So with those two things in mind I am continuing development off of 5.3.3.1 and trying to solve the bugs in it.
Now lets look at the issue in question.
H2Odk posted a fix for the code in question (is it tested or just a hypothetical fix?) that prompted 5.3.3.3, looking at his fix I see that the "var ds" was initialized both outside and inside the for loop, while the other two vars were initialized outside the for loop only. The changes were basically to delete the first 3 initialization and instead initialize all 3 within the for loop:
1. var cs = document.createElement("div");
Was relocated into the for function
2. var tops = 0;
was deleted, and instead the first incidence of using tops was changed from just "tops = stuff" to "var tops = stuff"
3. var ds = "";
was deleted on outer function so it isn't initialized twice.
OK so, first thing first, kudos on finding the issue.
Now as for the rewrite, when I was taught coding I was taught to initialize the variables at the beginning because, although the code is longer, someone can take a look at the start of the function and see a list of all the variables. Relocating all the variable initializing to within the for function makes the code take fewer lines, but breaks that rule.
For now I am just going to leave the original function as is and modify it to remove the second initializing attempt.
I would like to hear from others about what they think the proper syntax should be.
Unless, does js explode on a second initializing or does it merely act to modify the variable? If the latter then the actual issue isn't ds initializing twice but that the other two variables only initialize once (because they need resetting to the initial value?)... If so a cleaner solution is to initialize them once at the very beginning and then zero when appropriate.
I am going to test some variations of those to figure out which is which for 5.3.4. Also, we seem to all have completely different methods of naming, probably should agree on something. I changed 5.3.3 to 5.3.3.1 because all I did was added one single semicolon (as per GaryMcNabb) that was not even necessary for the function as it was just before the }. It is merely there so code doesn't explode later on. Regardless, it doesn't really matter as long as people increment it so we know what is newer...
Sweet baby tesla do we really need a package repository... I never set one up before but I will see about learning to do so... unless someone else beats me to it (IMG:[
invalid]
style_emoticons/default/tongue.gif)
This post has been edited by mrttao: Jul 26 2012, 00:33
HV STAT 5.2.2 (21th February 2011), HV Statistics, Tracking, and Analysis Tool
Jul 25 2012, 11:05
