Spellweaver's Talent Kitchen

Mediumunwell · Nov 18, 2021

Thank you so much for making this per my request. I'm sure it will be a great tool for many other map makers as well

SpasMaster · Nov 19, 2021

At first glance, this looks really cool and really customizeable. Great job.

I have a question.
If I understand correctly (after looking around the code for a bit), when a Talent has more than one dependency (like your "Coming of the Lambs"), currently you need to have both fulfilled in order to unlock it.

Is it possible to have 2 paths leading to a talent, but only needing one of the two dependencies to unlock it?

If not, is it something you can see being added to the system?

Thanks!

The_Spellweaver · Nov 19, 2021

It's great that you've noticed it, I appreciate you going through my code!

If not, is it something you can see being added to the system?

To answer your question, I haven't had this use case so I haven't implemented it. The way I would go about it would be a DependencyMode on the talent or something that would change the way CheckDependency logic works for that talent.

Specifically, the information is a number (required level) so I don't have much space to navigate in the baseline implementation. I've also had in plan to reserve the -1, -2 etc as flags. One usecase I've had was that I needed a disabled link no matter what, so I would set it to -1 (and handle it appropriately), then later programmatically change it to 0 if I want to unlock it (from perspective of dependencies no matter what).

So yeah it shouldn't be hard to add. Though like I said, it's a bit harder to design a user defined, out of the box boolean expression between the links lol
So saying two links need to be fulfilled + one of the other two is not something I'm thinking to add. It should be possible to do using Requirements callback + modifying talents' dependency values dynamically per talent! It's how I did it anyway.

The_Spellweaver · Nov 19, 2021

Made a different version of TalentTreeViewModel that allows multiple panels at the same time (such as 3 of them in WoW).
The problem is that they don't share talent points, and I'm not going to implement it. It needs to be done custom. I can implement additional functionalities (such as tree-wide callbacks) that will make it easier.

It will be able to be used in the Setup trigger.

Update:
I decided to make it a stub method, by default it keeps internal state of points, but it can be overriden in the implementation. In this case, it uses "lumber resource" as talent points.

JASS:

method GetTalentPoints takes nothing returns integer
    return GetPlayerState(this.ownerPlayer, PLAYER_STATE_RESOURCE_LUMBER)
endmethod

method SetTalentPoints takes integer points returns nothing
    call SetPlayerState(this.ownerPlayer, PLAYER_STATE_RESOURCE_LUMBER, points)
endmethod

method Initialize takes nothing returns nothing
    local STKTalent_Talent t
    call this.SetColumnsRows(3, 4)
    set this.title = "Shepherd"
    call this.SetTalentPoints(6)
    ...

The_Spellweaver · Nov 20, 2021

Okay I did some more features and I tried to make a representative example, to show people what can be made with it

I would say configuring this took me 2-3 hours

Functionally it works and everything. But I started feeling the drawbacks of jass itself, having tiny lags between actions like 0.05 seconds. During massive Cancel presses (like 30 or more unconfirmed points) it's even more present, maybe even 0.3-1 seconds or so.

The reason behind this could also be the amount of links (they are inefficient right now due to adding them just earlier and didn't optimize them, they also require some recursive calls due to how update works)

Confirming 30+ points doesn't make a delay however.
The "5 points per row" would have to be implemented through requirements.

In any case, I'm quite pleased how it works, in singleplayer it shouldn't be any trouble, and for few points here and there, shouldn't be any delay.
I have yet to start optimizing so I'll see how much performance I can get out of it.

I'm open to any questions.

SpasMaster · Nov 21, 2021

This is looking better and better by the minute!

Would you mind attaching this version of yours with the Druid example to your post to mess with and explore?

Aside from that, personally, I feel like you could have the system display by default the maximum points that can be put into a talent (eg; 0/5), just like how it is in your Druid example and unlike how it currently is by default where it just shows the current points.

The_Spellweaver · Nov 21, 2021

SpasMaster said:
This is looking better and better by the minute!

Would you mind attaching this version of yours with the Druid example to your post to mess with and explore?

Aside from that, personally, I feel like you could have the system display by default the maximum points that can be put into a talent (eg; 0/5), just like how it is in your Druid example and unlike how it currently is by default where it just shows the current points.

Thank you

I will attach this version when I've cleaned it up a bit. In this version I'm importing talent tree frames from RUID output although modified a bit, and using a configured Talent (button etc) views in an easy to change form. So I would like clean examples of that to be available too.

I can leave the current/maximum rank showing as default, though because it's dependency injected, one can always switch between the two view generating functions.

I've played around a bit with the performance and it seems Cancel is the biggest culprit and I'll see how to resolve it. I've kind of fixed the micro delay on putting points by delaying the update with a timer, but Cancel is so ingrained in the system that it'll be hard to delay it without messing up the system code.

I'll see what I can do ^^

Edit:
It seems the Cancel was causing lag cause the talent trees kept redrawing themselves every time Talent Points changed (which is a LOT during Cancel).
I've extracted the redraw logic to the outside of the viewmodel, so I'm passing an onViewUpdate that will update the views in a different trigger context (using a timer)

FlameofChange · Nov 22, 2021

What a lovely system, even if I found League of Legends and World of Warcraft heresy.
However I won't be of any technical advice, since it's a bit out of my league.

Kick King · Nov 22, 2021

Any chance you could port this to typescript or lua??

The_Spellweaver · Nov 22, 2021

Kick King said:
Any chance you could port this to typescript or lua??

I have a typescript version which I wrote this one from, but I didn't publish it cause there was no interest. Also there's a very early lua version of it somewhere in the code lab.

In any case, publishing it would require some cleaning, making examples etc.

I'll gladly share it if anyone's ready to play around with it.

Kick King · Nov 22, 2021

I would have A LOT of interest in a typescript version. This is exactly the type of system I'm looking for in my thing and everything is in typescript already. One thing I haven't spent any time in is custom UI stuff so this would save me a ton of time.

Coldgamer · Nov 25, 2021

This my friend is absolut amazing! Ty for the druid example!

Edit: Little question aside, how could I make like in wow a requirment of points set in a line? For example u have to set 5 points overall to unlock the 2nd line, 10 points to unlock the 3rd line and so on

The_Spellweaver · Nov 27, 2021

Coldgamer said:
This my friend is absolut amazing! Ty for the druid example!

Edit: Little question aside, how could I make like in wow a requirment of points set in a line? For example u have to set 5 points overall to unlock the 2nd line, 10 points to unlock the 3rd line and so on

Hey,
well you would do it using the Requirements functionality. Make a function that calculates minimum points based on row of that talent, then return an error if it's lower than necessary. I can help if you need, you can find me on discord.

Good luck!

Ugabunda · Dec 1, 2021

This looks amazing. Are you considering doing a GUI version?

Elprede · Dec 1, 2021

You can't access functions for frames in GUI, so most of the backbone code would be in custom script either way.

A GUI configuration would be possible with those set and get functions. And perhaps using real variables or triggers for events.

The_Spellweaver · Dec 1, 2021

Ugabunda said:
This looks amazing. Are you considering doing a GUI version?

Not any time soon, but I was considering/planning to make it possible to define talent trees in GUI, it would look something like this

Run trigger CreateTalentTree
// Talent 1
set variable TalentName = "Blah"
set variable TalentDescription = "Does this and that"
set variable OnActivate = Some GUI Trigger
set variable TalentCost = 2
set variable TalentX = 1
set variable TalentY = 2
Run trigger AddTalent
// Talent 2
set variable TalentName = "Blah2"
set variable TalentDescription = "Does this and that"
set variable OnActivate = Some other GUI Trigger
set variable TalentCost = 2
set variable TalentX = 1
set variable TalentY = 3
Run trigger AddTalent

Since that's the actual part where you "use" the system is by creating talent trees, and using the system should be easy.
But I don't intend on making the system customizable from GUI because the system itself is quite complex and it would do good for people to know code if they wanted to play around with how it works.

Coldgamer · Dec 2, 2021

Ugabunda said:
This looks amazing. Are you considering doing a GUI version?

I mostly also use GUI but take a look at the examples (especially the druid examp), in my opinion they are so easy to read/reproduce, even I could use this system :grin:

The_Spellweaver · Dec 2, 2021

Coldgamer said:
I mostly also use GUI but take a look at the examples (especially the druid examp), in my opinion they are so easy to read/reproduce, even I could use this system

Hey! That's the goal, at least out of the box, it should be clear, readable and easy to work with, regardless if you're an experienced coder or a casual GUI user.

Thanks :3

MatiasPalacios · Dec 17, 2021

This work on Multiplayer?

The_Spellweaver · Dec 19, 2021

MatiasPalacios said:
This work on Multiplayer?

Of course, it should. There's no point in it otherwise

Lt_Hawkeye · Mar 16, 2022

Is it possible using this system to have a specific talent point type per talent tree?

The_Spellweaver · Mar 16, 2022

Clanzion said:
Is it possible using this system to have a specific talent point type per talent tree?

Yes, in fact that's the default behavior. I had to change the example code so it would work like in WoW. Are you checking out the jass or ts version?

I'm willing to give an example but only if you ask

Lt_Hawkeye · Mar 16, 2022

The_Spellweaver said:
Yes, in fact that's the default behavior. I had to change the example code so it would work like in WoW. Are you checking out the jass or ts version?

I'm willing to give an example but only if you ask

Using the jass version, I see, I thought that wasn't the case due to the druid example showing the same talent point used for all three trees.

I also saw in the druid example, you manually set up the UI stuff? So will it be the case that if one were to want to change the background, one would need to manually do so using the UI natives? I'm not familar at all with the new UI natives.

The_Spellweaver · Mar 16, 2022

Clanzion said:
So will it be the case that if one were to want to change the background, one would need to manually do so using the UI natives?

You can definitely customize the way ui is rendered ("View" scripts) if you have the skills.

But if you only need to change the picture/background on the talent tree, then that's already part of the api. Check the druid example and see how it's done there

set this.backgroundImage = "balancebg.blp"

Lt_Hawkeye · Mar 17, 2022

Some suggestions to make this more versatile

1. Provide the ability to specific which talent tree and unit you wish to open for.
There are cases where a person may have multiple talent trees but not want them open at the same time.
For instance, an evolutionary tree would be fundamentally different from a ability talent tree.

2. Make the height and width of the talent tree configurable per talent tree.

d3v1lk1n · May 5, 2022

Hey this looks interesting, any chance to upload test map for non reforged users-1.31?
I tried porting it to test map but it crashes after 2 seconds in game.

The_Spellweaver · May 5, 2022

Hey, the jass or the typescript version? Because some natives I used might not be available in 1.31. I'll have to check. Sadly I haven't paid mind to backwards compatibility

d3v1lk1n · May 5, 2022

The_Spellweaver said:
Hey, the jass or the typescript version? Because some natives I used might not be available in 1.31. I'll have to check. Sadly I haven't paid mind to backwards compatibility

The jass version.

Wrda · Dec 8, 2022

Just mentioning here that I'm looking forward to posting my full review. I'm taking a while since it's a lot of code, but I've covered the STK map. Hopefully I'll post it tomorrow.

Wrda · Dec 10, 2022

Reference Leaks in TalentTreeViewModel -> CloseButtonClicked; CancelButtonClicked; ConfirmButtonClicked; TalentButtonClicked -> clickingPlayer variable.
Static method SetUpTriggersIfNeeded should be called in a "onInit" method than being constantly called at "create" static method. All of the above are included in the RUID version too.
It appears that in TalentViewModel the ActiveLinkTexture and TexturInactiveLinkTexture variables aren't uppercase as a whole, as the rest of the constants.
In TalentTree script, what exactly is the purpose of an empty library initializer? Is it for the user to utilize? (In RUID version too)
There's ActivateTalent method, following talent.onActivate, but no OnDeactivateTalent, nor talent.onDeactivate at all? (In RUID version too)
You can Simplify:

JASS:

method CheckDependencyKey takes integer requiredLevel, integer index, integer depIndex returns string
    local STKTalent_Talent talent = this.talents[index]
    local string errorText = null

    if (requiredLevel == 0 or talent == 0) then
        set errorText = null
    elseif (requiredLevel == -1) then
        set errorText = ""
    endif

    if (this.tempRankState[depIndex] < requiredLevel) then
        set errorText = errorText + this.talents[depIndex].name

        if (talent.maxRank > 1) then
            set errorText = errorText + " (" + I2S(requiredLevel) + ")"
        endif
    endif

    return errorText
endmethod

->

JASS:

method CheckDependencyKey takes integer requiredLevel, integer index, integer depIndex returns string
    local STKTalent_Talent talent = this.talents[index]
    local string errorText = null

    if (requiredLevel == -1) and not (requiredLevel == 0 and talent == 0) then //simplified the condition
        set errorText = ""
    endif

    if (this.tempRankState[depIndex] < requiredLevel) then
        set errorText = errorText + this.talents[depIndex].name

        if (talent.maxRank > 1) then
            set errorText = errorText + " (" + I2S(requiredLevel) + ")"
        endif
    endif

    return errorText
endmethod

JASS:

        // Grid starts from top left being (0, 0)
        method AddTalent takes integer x, integer y, STKTalent_Talent talent returns STKTalent_Talent
            return this.AddTalentRaw(x, y, talent)
        endmethod

        // Grid starts from top left being (0, 0)
        method AddTalentCopy takes integer x, integer y, STKTalent_Talent data returns STKTalent_Talent
            local STKTalent_Talent talent = STKTalent_Talent.createCopy(data)
            return this.AddTalentRaw(x, y, talent)
        endmethod

It starts from bottom left.
Talent script - It doesn't look a good idea for SetIcon method to have a prefix path. Not all icons have the same path logic (upgrades, passives, etc.)
It's not very clear what the whole API consists of, despite showing a good part of it in the example. Perhaps it would be nice to have a disabled custom script showing an API list (or a better way you have in mind).

RUID only version:
TalentTreeUIRuid - it would be nice if you fixed the indention here.
STKConstants - A lot of constants not following the standard rule, which you are already using. I think consistency is important.

JASS:

        private method AddTalentRaw takes integer x, integer y, STKTalent_Talent talent returns STKTalent_Talent
            local integer index = x + y * this.columns
            local STKTalent_Talent existing

            // If talent already exists
            if (this.talents[index] != 0) then

                // If talent is multirank, handle it differently
                if (this.talents[index].maxRank > 1) then
                    call this.talents[index].RemoveTalentFinalDescription()
                    call this.talents[index].SetLastRank(talent)
                    call this.talents[index].SetTalentFinalDescription()
                    call this.talents[index].UpdateMaxRank()
                else
                    call this.talents[index].SetLastRank(talent)
                    call this.talents[index].SetTalentFinalDescription()
                    call this.talents[index].UpdateMaxRank()
                endif
            else
                set this.talents[index] = talent
                set this.rankState[index] = 0
                call this.talents[index].UpdateMaxRank()
            endif

            if (talent.isLink) then
                set this.linkTalentIndex[this.linkTalentCount] = index
                set this.linkTalentCount = this.linkTalentCount + 1
            endif

            if (this.tempRankState[index] != talent.startingLevel) then
                set this.isDirty = true
            endif
            set this.tempRankState[index] = talent.startingLevel

            return talent
        endmethod

"existing" local variable is unused.
There are some "errors" in the examples. For example, "Balance", Improved Nature's Grasp's comments about rank keep being at rank 2. But that's a nitpick.
There seems to be a little spike when pressing the reset button after spending a considerable amount of talent points.

Overall, it's a pretty nice system to have, showing the beauty of the new natives. I would still like to see the major issues fixed first and then approve, although it's almost approvable.

verstefyren · Mar 10, 2023

Trying to validate map triggers on test map but I get "Trigger 'HeroPickAssignTalentTree' has been disabled due to errors. This also goes for GiveHeroTalentPoint and OpenTalentTreeUI. Mostly unknown compile errors/symbol not declared.

It worked a patch or two ago though.

Lt_Hawkeye · Mar 10, 2023

verstefyren said:
Trying to validate map triggers on test map but I get "Trigger 'HeroPickAssignTalentTree' has been disabled due to errors. This also goes for GiveHeroTalentPoint and OpenTalentTreeUI. Mostly unknown compile errors/symbol not declared.

It worked a patch or two ago though.

Which version are you using? Lua or Vjass
If Vjass, enable Jasshelper and vjass in the drop menue

verstefyren · Mar 10, 2023

Lt_Hawkeye said:
Which version are you using? Lua or Vjass
If Vjass, enable Jasshelper and vjass in the drop menue

Well, now I fell very stupid.. Thanks anyway

I hadn't turned on JassHelper. I just saw Vjass was enabled but greyed out. My mistake.
I have used this system earlier, very good indeed!

The_Spellweaver · Mar 23, 2023

Wrda said:
Reference Leaks in TalentTreeViewModel -> CloseButtonClicked; CancelButtonClicked; ConfirmButtonClicked; TalentButtonClicked -> clickingPlayer variable.

Fixed, I think.

Wrda said:
Static method SetUpTriggersIfNeeded should be called in a "onInit" method than being constantly called at "create" static method.

The reason why trigger creation is bound to constructor is mostly performance reasons, to avoid creating triggers in case there are no talent tree instances; i.e. there's no need for them. My assumption is that there might be 1-3 talent trees instantiated per player during the game so this function would not be called that many times, moreover, it by itself is very light (four null checks) after the first call.

I can make it a mandatory call in the setup though, it's probably a miniscule impact - I needed to explain reasons for this decision though.

Wrda said:
It appears that in TalentViewModel the ActiveLinkTexture and TexturInactiveLinkTexture variables aren't uppercase as a whole, as the rest of the constants.

I think I named them that way because they are (currently unchangeable) static properties of TalentViewModel class and not global constants (they are accessed with "thistype.X") - their value is set to the global constants defined in the Constants.j; it's used internally and is not part of the system's configuration (which is an assumption that something being a global constant would imply). I wanted to make that distinction clear when naming them I think.

Wrda said:
In TalentTree script, what exactly is the purpose of an empty library initializer? Is it for the user to utilize? (In RUID version too)

It's not there for user to initialize, it's part of the library internal code. There's no need for it, but it also explicity tells the reader that nothing happens on init.

Wrda said:
There's ActivateTalent method, following talent.onActivate, but no OnDeactivateTalent, nor talent.onDeactivate at all? (In RUID version too)

Talent Deactivation would be used in "Reset talents" mechanic, which was not supported. Talent class itself does support defining its onDeactivate, but it's not being triggered in any way. (no reset talents button or function after they've been activated). I've added the reset mechanic now, it's triggered by an ability in the STK example map.

Wrda said:

You can Simplify:

JASS:

method CheckDependencyKey takes integer requiredLevel, integer index, integer depIndex returns string
local STKTalent_Talent talent = this.talents[index]
local string errorText = null

if (requiredLevel == 0 or talent == 0) then
set errorText = null
elseif (requiredLevel == -1) then
set errorText = ""
endif

if (this.tempRankState[depIndex] < requiredLevel) then
set errorText = errorText + this.talents[depIndex].name

if (talent.maxRank > 1) then
set errorText = errorText + " (" + I2S(requiredLevel) + ")"
endif
endif

return errorText
endmethod

->

JASS:

method CheckDependencyKey takes integer requiredLevel, integer index, integer depIndex returns string
local STKTalent_Talent talent = this.talents[index]
local string errorText = null

if (requiredLevel == -1) and not (requiredLevel == 0 and talent == 0) then //simplified the condition
set errorText = ""
endif

if (this.tempRankState[depIndex] < requiredLevel) then
set errorText = errorText + this.talents[depIndex].name

if (talent.maxRank > 1) then
set errorText = errorText + " (" + I2S(requiredLevel) + ")"
endif
endif

return errorText
endmethod

Making this change would break the logic, since there's two outcomes based on its conditions. The smell here is that I am returning twofold information in this single string; Error exists/not exists, and the error message itself. It's unintuitive and risky, but due to jass' limitation, I'd have to write a bunch more (less performant) boilerplate as an alternative. I prefer this to that. CheckDependencyKey function is the most sensitive mechanic in the whole system, I would rather not touch it unless really needed.

Example, this is part of logic this change would break. (TalentTree.j line 344)
if (this.CheckDependencyKey(reqLevel, i, depIndex) == null) then
set level = level + 1
endif

Wrda said:
It starts from bottom left.

Tried to search for this text, couldn't find it. The issue seems to be already fixed in git, this is probably an issue in one of the example maps. I fixed it in the STK map.

Wrda said:
Talent script - It doesn't look a good idea for SetIcon method to have a prefix path. Not all icons have the same path logic (upgrades, passives, etc.)

The idea here was to minimize the effort and force consistency by adding the limit. It also makes the consumer/content code more clear and easier to read, which I think is very important. I added two methods SetIconEnabled and SetIconDisabled for those who need more control and flexibility.

Wrda said:
STKConstants - A lot of constants not following the standard rule, which you are already using. I think consistency is important.

Changed constants name to all follow UPPER_SNAKE_CASE

Wrda said:
There seems to be a little spike when pressing the reset button after spending a considerable amount of talent points.

Changed the way it's rendered, avoiding multiple recursive updates, performance increased a lot.

I didn't manage to get down all the points, will have to revisit RUID version individually.
All the changes can be seen in this pull request. Feature/review update 101222 by Gohagga · Pull Request #3 · Gohagga/STK_vJass

LovG · Mar 24, 2023

Wrda · Mar 27, 2023

Tried to search for this text, couldn't find it. The issue seems to be already fixed in git, this is probably an issue in one of the example maps. I fixed it in the STK map.

Must have been in STKTalentTree library (TalentTree script)

DRUID version only:
local player clickingPlayer variable is still leaking in TalentTreeViewModel script.

By the way, cancel functionality is functioning like the "reset" in the other version.

Performance seems better overall. Although there's no proper documentation, the examples show several features of the system enough to be a approved. A documentation is expected nonetheless.

Approved

The_Spellweaver · Mar 29, 2023

I updated the Druid map with all the new changes and features.

Wrda said:
local player clickingPlayer variable is still leaking in TalentTreeViewModel script.

With updated Druid map, this issue should be fixed now.

Wrda said:
By the way, cancel functionality is functioning like the "reset" in the other version.

I renamed "Reset" into "Cancel" within Druid map for consistency, and since "Reset" now refers to resetting the tree.

Wrda said:
A documentation is expected nonetheless.

I'm taking my time so I can finish it properly.

Also fixed RUID file indentation.

And this is it for now.

Radski · Mar 31, 2023

Amazing work! Been playing around with the system (Shepherd map) and haven't found anything really wrong with it, but noticed that talent descriptions on multi-rank talents dont reset when you reset the talents via Custom script: call STK_ResetUnitTalentTree(GetTriggerUnit()) It seems the way to reset the descriptions is to temporarily put enough points into the talent and then hitting the cancel button. Also noticed if a talent that is single rank depends on another talent (single or multi-rank), it wont show the required rank of dependency in brackets in the description. Minor things but still worth noting.

The_Spellweaver · Mar 31, 2023

Radski said:
Amazing work! Been playing around with the system (Shepherd map) and haven't found anything really wrong with it, but noticed that talent descriptions on multi-rank talents dont reset when you reset the talents via Custom script: call STK_ResetUnitTalentTree(GetTriggerUnit()) It seems the way to reset the descriptions is to temporarily put enough points into the talent and then hitting the cancel button. Also noticed if a talent that is single rank depends on another talent (single or multi-rank), it wont show the required rank of dependency in brackets in the description. Minor things but still worth noting.

Thank you very much for the bug report

Uploaded versions with documentation and I also fixed the "Reset" issue you are mentioning thanks to your insight.
I haven't looked into dependency error text message issue yet but I will at some point.

Radski · Apr 1, 2023

Sadly the latest version you just released has introduced a new bug, when you reset talents you get back twice as many talent points. Example you have 10 talent points spent and confirmed, then reset and you get 20 talent points back.

The_Spellweaver · Apr 1, 2023

Radski said:
Sadly the latest version you just released has introduced a new bug, when you reset talents you get back twice as many talent points. Example you have 10 talent points spent and confirmed, then reset and you get 20 talent points back.

Fixed

ThompZon · Apr 8, 2023

I tried to install it on a fresh map and noticed that Setup.j has this line:
STKTalentTreeViewModel_TalentTreeViewModel.create(Player(i), talentTreeView, GenerateTalentSlot, 1)
While the TalentTreeViewModel.create takes in 5 parameters, the last one is ViewChanged onViewChanged where it is defined as:
function interface ViewChanged takes TalentTreeViewModel ttvm, player watcher returns nothing

Basically, seems like the code on github result in compile error because of the above mentioned reason.

The_Spellweaver · Apr 8, 2023

ThompZon said:
I tried to install it on a fresh map and noticed that Setup.j has this line:
STKTalentTreeViewModel_TalentTreeViewModel.create(Player(i), talentTreeView, GenerateTalentSlot, 1)
While the TalentTreeViewModel.create takes in 5 parameters, the last one is ViewChanged onViewChanged where it is defined as:
function interface ViewChanged takes TalentTreeViewModel ttvm, player watcher returns nothing

Basically, seems like the code on github result in compile error because of the above mentioned reason.

Hey, github has code for ~3 different maps so I might not have tested it with latest changes.

Look at the Examples folder, they should have more correct versions of Setup.j

Or you can send the last param as -1

ThompZon · Apr 8, 2023

The_Spellweaver said:
Look at the Examples folder, they should have more correct versions of Setup.j

Or you can send the last param as -1

I only got so far as to follow instructions of

How To Install

and thought that the examples was "after install".
But thanks!

ThompZon · Apr 10, 2023

So, I've managed to get the example to run on Warcraft 3 version 1.31.1.
What's causing issues for this version is SetUpLink in TalentView, specifically the BlzFrameSetLevel call. From my understanding, this isn't really all that important to set and removing that line would make it compatible with 1.31.

Also, for some reason, there are a few commented // call this.ResetTalentViewModels(), and if you don't have the one in TalentClicked method, it doesn't update the UI as you click. Maybe that the others are good to have too, haven't really gotten that far yet.

But hey, neat system and it almost does exactly what I want, so cool!

What I want is 4 "choose 1 out of X" "talents", one unlocked per boss, but shouldn't be too hard to make it happen with this system

Fairly tiny thing, but there is a timer leak and not nulled timer in the setup.j when init calles GameBeginningSetup.

The_Spellweaver · Apr 16, 2023

ThompZon said:
So, I've managed to get the example to run on Warcraft 3 version 1.31.1.
What's causing issues for this version is SetUpLink in TalentView, specifically the BlzFrameSetLevel call. From my understanding, this isn't really all that important to set and removing that line would make it compatible with 1.31.

Also, for some reason, there are a few commented // call this.ResetTalentViewModels(), and if you don't have the one in TalentClicked method, it doesn't update the UI as you click. Maybe that the others are good to have too, haven't really gotten that far yet.

But hey, neat system and it almost does exactly what I want, so cool!

What I want is 4 "choose 1 out of X" "talents", one unlocked per boss, but shouldn't be too hard to make it happen with this system

Fairly tiny thing, but there is a timer leak and not nulled timer in the setup.j when init calles GameBeginningSetup.

Awesome to hear you are managing, I'm sorry it's not working out of the box. Not many users like you so it's not ironed out just yet.
I will see if I can make some changes you are suggesting next time I get to it.

I've been making a bunch of changes lately and while I did make the map examples work fully, I did not completely update the Setup.j files in git though it seems.

If you need any help with using or extending it (it's extendable

) throw me a direct message, or hit me up on hive discord (I'm more available there)

Kam · Jul 6, 2023

Outstandingly useful.

The_Spellweaver · Jul 12, 2023

Radski said:
Also noticed if a talent that is single rank depends on another talent (single or multi-rank), it wont show the required rank of dependency in brackets in the description. Minor things but still worth noting.

Sorry it took me a while, but I found and fixed this issue. I was in fact, checking max level of the talent in question to avoid displaying the "(rank)", instead of checking the max level of the talent it depends on. From what I've tested, it works now (updated maps and in git).

ThompZon said:
Basically, seems like the code on github result in compile error because of the above mentioned reason.

Now that I took a better look, you might have looked in the main branch which wasn't up to date, all the changes I've been doing were in the feature/reviewUpdate_101222. Though I've merged it now so it shouldn't happen again.

The_Spellweaver · Jul 17, 2023

A little tease, am finally working on save-load API. Also consolidated some things that were strange/different between Shepherd map and the druid one, I put some order into Setup.j files.

First and foremost, the API is for serializing and deserializing unit's one or more talent trees to and from a bit string. Then this bit string can be handled in one's map in whichever way you need/want/do already. In the example above, I also encoded them for easier testing.

I was considering to write out of the box integration with commonly used save-load systems, but I'd need to research those first.

Code:

-> function RegisterTalentTree takes integer id, TalentTreeFactory factoryMethod returns boolean
    Should be called on init for every type of TalentTree implementation. This must be done so that system knows which TalentTree to
    create for the unit when loading
    call STKSaveLoad_RegisterTalentTree(1, Shepherd.LoadCreate)
 
-> function LoadForUnit takes unit owner, string bitString returns nothing
    Takes the bitStream string, creates TalentTree instances for the unit, loads the ranks and activates talents
    call STKSaveLoad_LoadForUnit(udg_Hero, udg_BitStream)
 
-> function SaveForUnit takes unit owner returns string
    Takes the unit's TalentTree objects and creates a bit string with rank data
    call STKSaveLoad_SaveForUnit(udg_Hero)

I played around a lot with trying to reduce the size, and came up with this dynamic structure that depends on constant defined thresholds.
public constant integer TALENTTREE_ID_THRESHOLD_SMALL = 16
public constant integer TALENTTREE_ID_THRESHOLD_LARGE = 64
public constant integer TALENT_ID_THRESHOLD_SMALL = 32
public constant integer TALENT_ID_THRESHOLD_LARGE = 128
public constant integer TALENT_RANK_THRESHOLD_SMALL = 16
public constant integer TALENT_RANK_THRESHOLD_MEDIUM = 32
public constant integer TALENT_RANK_THRESHOLD_LARGE = 64
public constant integer TALENT_RANK_THRESHOLD_EXTRA = 256

So, there's 2 sizes for talent tree Id, 2 sizes for talent Id and 4 sizes for talent rank ids. This means as your map grows, so does amount of bits required to store various information in the save code. The idea is to keep it as small as possible while keeping backwards compatibility.
Also, it adjusts the size according to usage needs. Some people have 10 talents with 1000 ranks each, and some have 100 talents with 1 rank each. This makes it so that there's not too much overhead in either case.

Of course, the idea is that user sort of knows these thresholds/boundaries and to keep backwards compat, they should never be changed during development of the map. Of course, they can be changed, but that is very likely to break backwards compatibility. You could always play it safe and save everything as integers, doing it the old fashioned wc3 way, put 2,147,483,647 into those constants.

I have no idea how big the code would be at that point, probably around 7-10 times longer.

Cheers

Update: Ended up releasing it very soon. Major changes, likelihood of bugs. Sorry!

Spellweaver's Talent Kitchen

Spellweaver's Talent Kitchen (Map)

STK Druid Talents (Map)

How To Install​

Similar threads

How To Install