Now, if Parser-1.1 tries to load, it will load. Only if you don't have Parser-1.1 load will the compat layer actually be created, so now addons can (if they want to) replace Parser-1.1 with Parser-3.0 and not have any ill effects, while still having the option to run standard Parser-1.1.
Btw: The filter for unitIDs isn't really supposed to be all that effective for people other than player, but it doesn't _really_ need to be, granted, one could also have it catch all party members/raid members, and that'd work quite nicely.
While I understand the design of the current SW_FixLogStrings,
in my opinion, since SW_FixLogStrings is a stand-alone library, which job is to fix the problematic combat logs, then maintaining a list of problematic patterns, and actively fixes them probably can belong to its job too.
Can't get the trick with the frFR trick. Ckknight and myself already checked that their is no trouble with "|2".
"|2" is used both in GlobalStrings and string found in CombatLog. So as long as you deformat GlobalStrings, "|2" isn't a problem. It gets converted to appropriate string only at display time (like color codes and the like). Does anyone already have tested that this does NOT break things instead of fixing them ?
PS: "|2" stands for "de", which means "of", but have two spellings ("de" or "d'") in the same way that "a" and "an", e.g. "of Jerry" => "de Jerry", "of Adirelle" => "d'Adirelle".
Ckknight and myself already checked that their is no trouble with "|2".
There probably isn't on a pure parsing point of view.
I always found strange to read Baffe de Toto vous inflige 32 degats |2 Feu. in my combat log, though, so I'm glad that SW_FixLogStrings got rid of thoses.
This is pure cosmetics.
Note that all the current transformations tries to keep the combat string readable.
Quote from Adirelle »
Does anyone already have tested that this does NOT break things instead of fixing them ?
Been using this for quite some time, and I've never had an issue with it. So I'm pretty sure it does not break anything.
Edit: I should add here something about BigWigs (and probably other similar addons) that match combat strings to detect Boss abilities. A lot of these patterns have been modified to work with or without the change to pattern made by (SW_)FixLogStrings. For instance, patterns like Flammes \195\169chauffantes de Tranchetripe l'Indompt\195\169... where changed to Flammes \195\169chauffantes .+ Tranchetripe l'Indompt\195\169
A solution is to have all those addons set FixLogStrings as an OptDeps, so it is guaranteed to load before those addons.
But, if we use our own FixLogStrings, SWStats use its own FixLogStrings, then the first one to be loaded will still break the second one, so before we actually use this, I think we should ask author of SWStats first.
But, if we use our own FixLogStrings, SWStats use its own FixLogStrings, then the first one to be loaded will still break the second one, so before we actually use this, I think we should ask author of SWStats first.
Maybe adding
## LoadWith: SW_FixLogStrings
might be enough to make sure that FixLogStrings performs it's task before SW_FixLogStrings.
might be enough to make sure that FixLogStrings performs it's task before SW_FixLogStrings.
It should work, but still I think the original author probably should be notified about this, since the code comes from him.
Also, does modifying combat logs taint anything? I do have a private mod which modifies combat log and my UI seems to work fine, but I'm not 100% sure on that.
No. I don't think modifying these variable is supported, the fact is that it works, but the code that format the combat log strings is either taint-proof or more likely written in C.
I always found strange to read Baffe de Toto vous inflige 32 degats |2 Feu. in my combat log, though, so I'm glad that SW_FixLogStrings got rid of thoses.
In theory any addon will be given the string containing "|2". But as soon as it is put into any UI fontstring, it is displayed as "de" or "d'". I'm wondering how you can see it ? (Unless all "|" are escaped before displaying).
But as long as we can ensure it doesn't break anything, I don't really care.
No. I don't think modifying these variable is supported, the fact is that it works, but the code that format the combat log strings is either taint-proof or more likely written in C.
hmm, add FixLogStrings to OptDeps of the stand alone parsers then.
This won't work with embedded parser, but most people shouldn't need this fix anyway, those who wants it can/should dis-embed the parsers. So I think it is good enough in the current form.
hmm, add FixLogStrings to OptDeps of the stand alone parsers then.
I've put FixLogStrings (without the "|2" transformation) on trunk. I'll do some testing with and without SW_Stats at home before, and if I don't see any issue, I'll add the Optional Dependency.
Okay, I actually put in a fixlogstrings thing into Parser-3.0, but it's currently not activated due to political reasons (rophy thinks it's bad karma).
It's very similar to SW_FixLogStrings, only a bit faster.
Basically, the reason it'd be bad to update is that if some other third-party parser read the global strings, it was fixed, and the event run, said parser wouldn't read it possibly. This is solved, of course, by running the fix before anything else. So, it has the potential to break other addons, but if it isn't done, then some strings are ambiguous and the parsing is fucked anyway.
Okay, I actually put in a fixlogstrings thing into Parser-3.0, but it's currently not activated due to political reasons (rophy thinks it's bad karma).
It's very similar to SW_FixLogStrings, only a bit faster.
Basically, the reason it'd be bad to update is that if some other third-party parser read the global strings, it was fixed, and the event run, said parser wouldn't read it possibly. This is solved, of course, by running the fix before anything else. So, it has the potential to break other addons, but if it isn't done, then some strings are ambiguous and the parsing is fucked anyway.
I dont think I'm the only one who agree an addon should avoid breaking other addons whenever possible.
With an external FixLogStrings, if you want to fix your combat log, you use it; if you don't want, don't use it, nothing breaks for not using it, no change required except adding OptionalDeps.
I don't see why it should be included in Parser-3.0, when Parser-3.0 needs nothing from FixLogStrings to work.
Sure, the same can be done in Parser-3.0 and adding OptionalDeps to all addons, but if everyone think including the fix in their own addon is a good idea, then SWStats will have an inline FixLogString, ParserLib has an inline FixLogString, Parser-3.0 has an inline FixLogString, along with 100 other addons which parse combat logs, now who should load first?
I don't think "an addon should not break other addon whenever possible" is poltical,
"if you don't follow my standard, I kill you" looks more political to me.
Rophy, what you don't seem to understand is that if, let's say, Parser-1.1, Parser-3.0, and SWStats all have their own FixLogString implementation, that it doesn't matter which loads first, as long as the job is done properly. If Parser-3.0 loads, then SWStats' fixer doesn't actually do anything. Likewise if SWStats is loaded first.
Yes, I was wrong about two FixLogStrings breaking each other. The fix will not be applied when it isn't necessary, so the FixLogStrings got called later should have no effect.
Btw: The filter for unitIDs isn't really supposed to be all that effective for people other than player, but it doesn't _really_ need to be, granted, one could also have it catch all party members/raid members, and that'd work quite nicely.
I have committed FixLogStrings here : http://dev.wowace.com/wowace/branches/FixLogStrings/Jerry/FixLogStrings.
Thoughts ? Comments ?
Can't get the trick with the frFR trick. Ckknight and myself already checked that their is no trouble with "|2".
"|2" is used both in GlobalStrings and string found in CombatLog. So as long as you deformat GlobalStrings, "|2" isn't a problem. It gets converted to appropriate string only at display time (like color codes and the like). Does anyone already have tested that this does NOT break things instead of fixing them ?
PS: "|2" stands for "de", which means "of", but have two spellings ("de" or "d'") in the same way that "a" and "an", e.g. "of Jerry" => "de Jerry", "of Adirelle" => "d'Adirelle".
There probably isn't on a pure parsing point of view.
I always found strange to read Baffe de Toto vous inflige 32 degats |2 Feu. in my combat log, though, so I'm glad that SW_FixLogStrings got rid of thoses.
This is pure cosmetics.
Note that all the current transformations tries to keep the combat string readable.
Been using this for quite some time, and I've never had an issue with it. So I'm pretty sure it does not break anything.
Edit: I should add here something about BigWigs (and probably other similar addons) that match combat strings to detect Boss abilities. A lot of these patterns have been modified to work with or without the change to pattern made by (SW_)FixLogStrings. For instance, patterns like Flammes \195\169chauffantes de Tranchetripe l'Indompt\195\169... where changed to Flammes \195\169chauffantes .+ Tranchetripe l'Indompt\195\169
If FixLogStrings is loaded after an addon which parse the GobalStrings then it might break the addon.
as an example see http://ace.pastey.net/45382
A solution is to have all those addons set FixLogStrings as an OptDeps, so it is guaranteed to load before those addons.
But, if we use our own FixLogStrings, SWStats use its own FixLogStrings, then the first one to be loaded will still break the second one, so before we actually use this, I think we should ask author of SWStats first.
This is not worse than the current state where it does not work for frFR, because Parser-3.0 is loaded before SW_Stats.
At least the job's done if FixLogStrings is an optional dependency of Parser-3.0 and ParserLib. SW_FixLogStrings will then be a no-op.
Maybe adding
might be enough to make sure that FixLogStrings performs it's task before SW_FixLogStrings.
It should work, but still I think the original author probably should be notified about this, since the code comes from him.
Also, does modifying combat logs taint anything? I do have a private mod which modifies combat log and my UI seems to work fine, but I'm not 100% sure on that.
No. I don't think modifying these variable is supported, the fact is that it works, but the code that format the combat log strings is either taint-proof or more likely written in C.
In theory any addon will be given the string containing "|2". But as soon as it is put into any UI fontstring, it is displayed as "de" or "d'". I'm wondering how you can see it ? (Unless all "|" are escaped before displaying).
But as long as we can ensure it doesn't break anything, I don't really care.
Now, I admit that "|2" has it's use, and probably should not be changed in FixLogStrings.
hmm, add FixLogStrings to OptDeps of the stand alone parsers then.
This won't work with embedded parser, but most people shouldn't need this fix anyway, those who wants it can/should dis-embed the parsers. So I think it is good enough in the current form.
I've put FixLogStrings (without the "|2" transformation) on trunk. I'll do some testing with and without SW_Stats at home before, and if I don't see any issue, I'll add the Optional Dependency.
Just stop doing do that :D
It's very similar to SW_FixLogStrings, only a bit faster.
Basically, the reason it'd be bad to update is that if some other third-party parser read the global strings, it was fixed, and the event run, said parser wouldn't read it possibly. This is solved, of course, by running the fix before anything else. So, it has the potential to break other addons, but if it isn't done, then some strings are ambiguous and the parsing is fucked anyway.
I dont think I'm the only one who agree an addon should avoid breaking other addons whenever possible.
With an external FixLogStrings, if you want to fix your combat log, you use it; if you don't want, don't use it, nothing breaks for not using it, no change required except adding OptionalDeps.
I don't see why it should be included in Parser-3.0, when Parser-3.0 needs nothing from FixLogStrings to work.
Sure, the same can be done in Parser-3.0 and adding OptionalDeps to all addons, but if everyone think including the fix in their own addon is a good idea, then SWStats will have an inline FixLogString, ParserLib has an inline FixLogString, Parser-3.0 has an inline FixLogString, along with 100 other addons which parse combat logs, now who should load first?
I don't think "an addon should not break other addon whenever possible" is poltical,
"if you don't follow my standard, I kill you" looks more political to me.