Master Level Overhaul

[dargon]

It may not send the skills livelike but to save the trouble of the bars couldmt you do order based on spellID?

[Leodagan]

thanks for the idea dargon

But SpellID order has nothing to do here, Spell ID isn't even sent to client in the Skill Update

Client UI (quickbar) have two way of referencing a click-able skill : if it's an ability/style/Spec/Hybrid it store the index number of the skill list sent by server, if it's a List Caster skill it stores the lineindex (server order dependent too !!) and the level of the spell in this line (thus a pure caster can't have two spell at the same level in the same line !!)

As the client use a lot of index based on how the server provided the skills this forces us to keep some kind of "order" in how we send skill, this doesn't break anything around styles as they are not replacing themselves like hybrid spells and are easily appended to the end of the style list.

The difficulty around Hybrid is that it use some kind of "group by" method (even implemented by hand like Mark did) and this method doesn't keep order of data, without ordering we have to sort on something that will "looks like" it's appended to the client given skill list...

[Leodagan]

I couldn't loose too much time on this...

I logged some live packet around a bard that I slowly leveled in tutorial, some level, then some training, then some level, then some training, some re logging to check if the order is kept between re log...

Here are the results for anyone interested :

[Leodagan]

Ok, in fact it's stupidly simple !!

If there is a new spell/style/ability to send, check if it replace an existing one, if not, append to the list and don't matter !

it must never change the initial list up to the end of the playing session, then when re logging the order is completely changed it just looks like spec spell at are the end of the list ordered by line and appearing order, and client must retrieve its data through the .ini file...

I think the order "in" the Line must be kept no matter what, this order don't need to be consecutive entry, thus you can append to the skill list but in the line order ! I don't know if it's clear anything that should not go to the end of line should REPLACE an element and not change order, this way we keep the order in the line it will be easy to retrieve the full line list using a simple .Where(e => e.SpellLine == x) (the where method keeps Order !)

Client must save its icon with index compared to the ordered displayed line (hybrid are displayed by spell level...) but save only happen on quit, when crashing a client mostly don't save its icon... this is how it can keep up with server list during gameplay just with new record or replacing ones, and save it all to an other format on hard drive...

[Tolakram]

You just need to implement it exactly like I did if you insist on changing it. I'm not sure why GetUsableSpells needs re-written. That required a lot of work and a lot of modifications to the spell table to get it to work correctly. Why not continue to use spellgroup?

[Leodagan]

I'm stil using all what you did, but I needed the method to use my new "specialization" which are the object which will provide the "GetUsableSpells" method with player collections !

GamePlayer shouldn't be "centered" around rules or network packet so I'm taking some part out of it, I still need your code but can't rely on it as I'm using new collection

Edit :

Here you see, it's reduced to some lines, but this is exactly your implementation written in LINQ

Code: Select all

~~ "lib" is a list of spell available, "take" is the hardcoded number of spell to keep ~~ var firstBetterSpellNoGroup = lib.Where(item => item.Group == 0) .GroupBy(item => new { item.SpellType, item.Target, item.IsAoE, item.IsInstantCast, item.HasSubSpell }) .Select(ins => ins.OrderByDescending(it => it.Level).ThenByDescending(it => it.ID).Take(take)); var firstBetterSpellGroup = lib.Where(item => item.Group != 0) .GroupBy(item => item.Group) .Select(ins => ins.OrderByDescending(it => it.Level).ThenByDescending(it => it.ID).Take(take)); // sort by reverse level for multiple version var firstBetterSpellFinal = firstBetterSpellGroup.SelectMany(el => el).Union(firstBetterSpellNoGroup.SelectMany(el => el)) .Where(item => item != null).OrderByDescending(item => item.Level).ToList(); // Get Appearance Order // not group base var baseOrderNoGroup = lib.Where(item => item.Group == 0) .GroupBy(item => new { item.SpellType, item.Target, item.IsAoE, item.IsInstantCast, item.HasSubSpell }) .Select(ins => ins.OrderBy(it => it.Level).ThenBy(it => it.ID).Take(take)); // group based var baseOrderGroup = lib.Where(item => item.Group != 0) .GroupBy(item => item.Group) .Select(ins => ins.OrderBy(it => it.Level).ThenBy(it => it.ID).Take(take)); // Join and sort... var baseOrderFinal = baseOrderNoGroup.SelectMany(el => el).Union(baseOrderGroup.SelectMany(el => el)) .Where(item => item != null).OrderBy(item => item.Level).ThenBy(item => item.ID).ToList(); int countbase = 0; while (countbase < baseOrderFinal.Count) { // replace each spell of order with their best equivalent Spell sp = baseOrderFinal[countbase]; Spell find = firstBetterSpellFinal.Where(od => ((od.Group == 0 && sp.Group == 0) && (od.SpellType == sp.SpellType && od.Target == sp.Target && od.IsAoE == sp.IsAoE && od.HasSubSpell == sp.HasSubSpell && od.IsInstantCast == sp.IsInstantCast)) || (od.Group != 0 && od.Group == sp.Group)).FirstOrDefault(); if (find != null) { baseOrderFinal[countbase] = find; firstBetterSpellFinal.Remove(find); countbase++; } else { baseOrderFinal.RemoveAt(countbase); } }

Edit incoming, in fact there is an error :s

[Tolakram]

Linq is so flipping unreadable.

[Leodagan]

It's so easy to code with LINQ

It's like having SQL on any collection

And Here I use the procedural form, I think it's way more readable than the query mode !

I finally found what I was doing wrong, thanks for bearing with me, I'll update the code snippets !

Everything is working fine now, hybrid spells, list spells, ability, styles, everything

I read a lot of old posts talking about getting rid of all these trouble with hybrid, here my method use cache to be sure It ALWAYS append new skills or spells, and existing skills are sorted by specs and/or line to match the exact order it should be as we query the "career order", this make the replacement of the spells automatic as we always obey the order of the subline !!

Well for me I'm done with these trouble, I can walk on Champion / RA / ML implementation and tests through the new specs handler

The logic is to group spell by the method you want, either group == group or by using a "Struct" of properties that match exactly what you used (Type, Target, isAoE, isInstant, HasSubSpells), in grouping method we can use "Aggregation" function like in SQL, thus selecting the 2 firsts spell ordered by MaxLevel, or ordered by MinLevel to get the famous "Appearance" order !

Ordering by minlevel make sure this match the progression of the player, the last loop is a bit ugly but it's only to sort out the best spells according to the minlevel positions...

[Leodagan]

Current Storm Have absolutely no Master Level implementation in Character Career...

It seems as the Master Level quests were never finished Storm didn't implement anything to acquire ML ?

Well that's fine with storm that will be able to use a "white table" for beginning, but some things may get dirty for people using previous "ML NPC" for handing ML to players...

So actually player.ML is a byte value used to set player ML Line, there is no constant anywhere describing what this byte value should mean, there are a lot of hard coded methods that expect ML level to be max "10" but nothing around ML Line, for me this value could reflect an hard coded enum to target specific ML "spec" but with my new way of handling spec this doesn't help me to tell appart which class can be which ML...

So I'm going to implement each ML line as Database Career, they'll be earned at level 40 and return always "Player.MaxLevel" Skills to be sure everything is set to Level 50 (could give a short advantage to players earning ML's early, but anyway on storm nobody earns ML for now...) ML Line value will be an index on which career should be enable (so if you have 2 ML choice, 0 will target the first indexed spec and 1 the second indexed spec, Order can be enforced with Database ID to stay consistent) and ML Level will trigger how much of ML Specialization Skills to enable...

Using an index as ML line will allow to use any custom database made "ML career" if some shard want to enable ML choice between multiple custom lines or expand the way ML work this wouldn't matter as long as you don't have more than 255 choices, value will stay consistent as long as database isn't too heavily updated, you should hand over ML respec to player if you happen to edit too much data and allow for changing "ML Line" to the right new index...

Well with this I'm back to spend some time into "Database" building

[Leodagan]

After some more live testing I have other features to work on...

Abilities seems to be saved to character in "order", I hadn't anticipated these and it explains the further feature I noticed :

Master Level are sent to player as "List Caster Spells" but can contains Styles or Ability (passive or active...), newer client version use a special index flag to display according tooltip, which means they AREN'T spell container like we use for Spell "StyleHandler"

It's easy to put any "Skill" inside a List Caster Spell List, it just need a Level and an Icon, when receiving the according "Use Request" from player UI it send indifferently the lineindex and linelevel whatever it is (the client don't know what is behind the icon in fact) and I can easily switch the handling based on the skill Type at index we sent to client !

For Abilities order saving it's another story, this explain why no one has moved Realm Abilities from Serialized Abilities Character field...

Any way I intend to use the Serialized Abilities field but as "Order" only field, this won't be the field used to enable or disable skill and I will try anything to make it "not human" editable, this will give the real use of Serialized Realm Abilities to really handle the "granted" skills, I want this to enable easy Realm Abilities Respec by deleting the whole entry...

Actually doing a player Skill respec is pretty easy by deleting the Serialized Spec field, all "Career" are reset to the player current level anyway, and every trainable spec are added back to level one, the auto spec point method checks that player is missing points and refund everything

As ML line / Level is handled in another field, and Champion Lines / Level will mostly be used in Serialized SpellLines (it's the only use I can see to this field now, spec handles them otherwise...) I like having field safe to be reset in player database, Live server don't forces Player respec on some version change for nothing there is a lot of "rebuild" to do when changing some values !

PS : that make me think that working with such an old fashioned Client is really a pain in the ass, And broadsword may be coming with heavy upgrades, They don't have the same constraints as keeping version compatibility like we do in DOL, virtually Keeping this huge range of client compatibility is really a challenge !

[Graveen]

But basically we could also - in the way we were forced to do with 1.109 - state to a compatibility version.

With OF, NF, NNF, docks, we have enough reasons to force a defined version.

The main challenge is to have a working 1.115

anyway, we also could push conditionnal compilation feature for version X or Y, thus focusing on a defined version, such as a
#ifdef VERSION1115
#else
#endif

but i prefer simply sticking to lastest supported revision, to be honest.

Of course it means older revision would not benefits all bugfixes, but even for a classic pre-168 server, i doubt this is interesting to not stick the latest client.

[Leodagan]

Well actually I'm sure nobody can build a DOL server compatible with 1.68 Client (from heavy position code update) maybe with some tuning we could get back this kind of old client to work, but this isn't even my work, I suspect some identifier got their range improved around version 1.80 / 1.95 and packet code of older client would not be able to handle newer region/zone id or other features (like quest icon, maps, Warlock chambers)...

1.95 / 1.109 / 1.110+ are still working with most of DOL revision (excepting a good tooltip implementation), the updates of UI is only around some Maps/Quest Identifier/Trainer Window/Market Search for the most useful feature of recent client revision...

Item are not tooltip'ed for now which prevent from having an headache on "how" we are going to handle unique item or "numeric" item template id, but Champion Training Windows may get an upgrade soon as Broadword announced new skills and new CL level... (well I don't expect breaking feature as CL Line reproduce the behavior of Spec Line but much shorter !)

The assets improvement of the client is another story, it was pretty much "stable" after New Frontiers and Laby (1.95 if I remember right ?) and as a community emulator we can't stay away from supporting those beloved extensions

OF / NF "Switching" involves Data management and maybe some Keep Manager tuning, NF / NNF switching won't survive long except for shard Admin that handles their own "rollback" patches, NNF changed geometry a bit more importantly than the few differences we could see in Old Frontier Portal Keep Geometry like on Uthgard, the new lightning system being the last step to break most of Assets Shards could use efficiently for their frontiers...

Actually I think an OF Shard will have to use new Keep Skins shortly, or handle their patch, NF or NNF Shard will just need to refresh their keep data from a packet log session, and adapt the rules of their server to the few geometry changes...

Following up to date assets even if we are still roll-backing the client DLL will be easier

But I fear that more changes could come in line, and some part of the client is pretty tricky to get working with our emulator, actually "Tooltip" based on numeric identifier forces us to keep all our game object based on a data set that keeps ID in good shape...

We'll have to get rid sooner of later of totally "dynamic" game object that interact with player UI, even short life item could need to get a steady tooltip id. That's a big "feature" breaking from previous DOL revision...

[Leodagan]

I'm actually stalled on this improvement.

I have a huge and ugly memory leak around Hybrid Spell list...

Even if I'm not looking for best performances around this part of code I can't have this kind of bug left unattended !

[Graveen]

The 1.11x support is a nightmare...

Wait.... Halloween effect ?!?!

[Leodagan]

Well it's pretty weird indeed :

I've pin-pointed the leak to this method

Code: Select all

/// <summary> /// Get Summarized "Hybrid" Spell Dictionary /// List Caster use basic Specialization Getter... /// This would have pretty much no reason to be used by GameLiving... (Maybe as a shortcut to force them to use their best spells...) /// </summary> /// <param name="living"></param> /// <returns></returns> protected override IDictionary<SpellLine, List<Skill>> GetLinesSpellsForLiving(GameLiving living, int level) { // An hybrid dictionary is composed of a spellline "base" // SpecLine and Baseline are mixed in spellline named "base" // baseline are displayed first (improvement are easier this way) // specline are displayed secondly (ordered in appareance order) // some class/spelline are allowed to display the "2 -best" spell // this is hardcoded in AllowMultipleSpellVersions... Dictionary<SpellLine, List<Skill>> buffer = new Dictionary<SpellLine, List<Skill>>(); List<SpellLine> lines = GetSpellLinesForLiving(living, level); foreach (SpellLine ls in lines) { // buffer shouldn't contain duplicate lines if (buffer.ContainsKey(ls)) continue; // Add to Dictionary List<Skill> finalList; finalList = new List<Skill>(); buffer.Add(ls, finalList); IEnumerable<Spell> lib = SkillBase.GetSpellList(ls.KeyName).Where(item => item.Level <= ls.Level); int take = 1; if ((living is GamePlayer) && AllowMultipleSpellVersions(ls, (GamePlayer)living)) { // Get 2-First Better Spell for each type take = 2; } IEnumerable<IEnumerable<Spell>> firstBetterSpellNoGroup = lib.Where(item => item.Group == 0) .GroupBy(item => new { item.SpellType, item.Target, item.IsAoE, item.IsInstantCast, item.HasSubSpell }) .Select(ins => ins.OrderByDescending(it => it.Level).ThenByDescending(it => it.ID).Take(take)); IEnumerable<IEnumerable<Spell>> firstBetterSpellGroup = lib.Where(item => item.Group != 0) .GroupBy(item => item.Group) .Select(ins => ins.OrderByDescending(it => it.Level).ThenByDescending(it => it.ID).Take(take)); // sort by reverse level for multiple version List<Spell> firstBetterSpellFinal = firstBetterSpellGroup.SelectMany(el => el).Union(firstBetterSpellNoGroup.SelectMany(el => el)) .Where(item => item != null).OrderByDescending(item => item.Level).ToList(); // Get Appearance Order // not group base IEnumerable<IEnumerable<Spell>> baseOrderNoGroup = lib.Where(item => item.Group == 0) .GroupBy(item => new { item.SpellType, item.Target, item.IsAoE, item.IsInstantCast, item.HasSubSpell }) .Select(ins => ins.OrderBy(it => it.Level).ThenBy(it => it.ID).Take(take)); // group based IEnumerable<IEnumerable<Spell>> baseOrderGroup = lib.Where(item => item.Group != 0) .GroupBy(item => item.Group) .Select(ins => ins.OrderBy(it => it.Level).ThenBy(it => it.ID).Take(take)); // Join and sort... IEnumerable<Spell> baseOrderFinal = baseOrderNoGroup.SelectMany(el => el).Union(baseOrderGroup.SelectMany(el => el)) .Where(item => item != null).OrderBy(item => item.Level).ThenBy(item => item.ID); foreach (Spell sp in baseOrderFinal) { // replace each spell of order with their best equivalent int index = firstBetterSpellFinal.FindIndex(od => ((od.Group == 0 && sp.Group == 0) && (od.SpellType == sp.SpellType && od.Target == sp.Target && od.IsAoE == sp.IsAoE && od.HasSubSpell == sp.HasSubSpell && od.IsInstantCast == sp.IsInstantCast)) || (od.Group != 0 && od.Group == sp.Group)); if (index > -1) { finalList.Add(firstBetterSpellFinal[index]); firstBetterSpellFinal.RemoveAt(index); } } } return buffer; }

If someone has any clue on where there could be a leak, for my part it's voodoo (even if I wrote this myself...)
Sub-methods have been tested in this part of code and they don't leak