Skip to content

Conversation

@LordMidas
Copy link
Member

@LordMidas LordMidas commented May 13, 2025

  • feat: add ability to dynamically generate text based on object field such as Name for skills and items, using Obj/Field notation in shorthand.
  • feat!: require subfolder path for perks, items, skills
    • This allows files with the same filename to exist.
    • For perks this is the scripts/skills/perks/ folder so they remain unchanged. Shorthand: [Anticipation|Perk+perk_anticipation] or [Obj/Name|Perk+perk_anticipation]
    • For items it is the scripts/items/ folder so now we have weapons/knife instead of knife. Shorthand: [Knife|Item+weapons/knife] or [Obj/Name|Item+weapons/knife]
    • For skills it is the scripts/skills folder so now we have effects/stunned_effect instead of stunned_effect. Shorthand: [Stunned|Skill+effects/stunned_effect] or [Obj/Name|Skill+effects/stunned_effect]
    • Delete SkillObjectsByFilename table. Skill objects are instantiated on demand during a nested tooltip query.
    • No longer instantiate all items at the start of the game. Instead they are instantiated and added to ItemObjectsByFilename on demand when a nested tooltip is queried for them.

@LordMidas LordMidas requested a review from TaroEld May 13, 2025 21:20
@LordMidas LordMidas linked an issue May 13, 2025 that may be closed by this pull request
@LordMidas LordMidas changed the base branch from development to better-item-handling-for-nested-tooltips May 13, 2025 21:22
if (ret == null)
{
skill = ::MSU.NestedTooltips.SkillObjectsByFilename[_data.Filename];
skill = ::new(scriptPath);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we do that caching to items but not skills, why?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't seem necessary and just hogs memory. For items it is necessary because we need to have the item available for the nested tooltips of skills which want to refer to that item e.g. reload_bolt which needs an item.

Copy link

@Suor Suor May 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, doesn't make sense. Why don't you create an item on demand like everything else?

Copy link
Member Author

@LordMidas LordMidas May 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do that in this new system, we create it on demand. But when we create it, we also add it to this table with a strong reference so it always exists. We need it to persist outside this scope as well because the skill's tooltip will need to refer to that item. If we just create it on demand and return it, the object will be garbage collected and we won't have access to it anymore. See how our hook in tooltip_events.nut where we need to access an item if necessary, so the item needs to exist.

We don't need to do this for skills i.e. they don't need to exist outside that scope as they are never accessed outside it.

@Suor
Copy link

Suor commented May 14, 2025

Why we need all this dummy player, fake item and stuff? Why don't use the real thing?

@LordMidas
Copy link
Member Author

Why we need all this dummy player, fake item and stuff? Why don't use the real thing?

This is a long topic but I'll try to summarize it:

  • Many things require to have a container in order to generate a proper tooltip. Therefore, for nested tooltips of skills we need to assign them to a container.
  • For the tooltips of skills of items, those skills need to have a reference to an item so that skill.getItem() works properly. Therefore, we need to equip the item on a character in order to generate the proper skills which are generated through that item's onEquip function.
  • Active skills in particular need this because there needs to be an update on the character's skill_container in order for the weapon to add its damage and other things to the character which are then necessary for the tooltip of the skill.

Without this DummyPlayer most of the framework will not work because skills will be hanging without a container and items will be hanging without being equipped.

This is just a brief summary of it, going into much detail of it will be probably too long here.

@Suor
Copy link

Suor commented May 14, 2025

I understand that you need a player, but a skill usually already has one, why not use that?

@LordMidas
Copy link
Member Author

I understand that you need a player, but a skill usually already has one, why not use that?

For example if I want to give the nested tooltip of stunned_effect inside a random tooltip, this stunned_effect is not present on any player. So, we temporarily assign it the container of the DummyPlayer and generate its tooltip. For the stunned_effect this isn't a problem because it doesn't need container in getTooltip so it will work even without the DummyPlayer, but there are many skills that require a container or an actor e.g. reload_bolt.

@LordMidas LordMidas force-pushed the dynamic-field-from-nested-object branch from 2bda3d0 to 51020e4 Compare November 16, 2025 23:22
- This allows files with the same filename to exist.
- For perks this is the scripts/skills/perks/ folder so they remain unchanged. Shorthand: [Anticipation|Perk+perk_anticipation] or [Obj/Name|Perk+perk_anticipation]
- For items it is the scripts/items/ folder so now we have weapons/knife instead of knife. Shorthand: [Knife|Item+weapons/knife] or [Obj/Name|Item+weapons/knife]
- For skills it is the scripts/skills folder so now we have effects/stunned_effect instead of stunned_effect. Shorthand: [Stunned|Skill+effects/stunned_effect] or [Obj/Name|Skill+effects/stunned_effect]

- Delete SkillObjectsByFilename table. Skill objects are instantiated on demand during a nested tooltip query.
- No longer instantiate all items at the start of the game. Instead they are instantiated and added to ItemObjectsByFilename on demand when a nested tooltip is queried for them.
@LordMidas LordMidas force-pushed the dynamic-field-from-nested-object branch from 51020e4 to c4e0af2 Compare November 16, 2025 23:23
@LordMidas LordMidas force-pushed the better-item-handling-for-nested-tooltips branch from 919d995 to c0e7d6d Compare November 16, 2025 23:24
@LordMidas LordMidas force-pushed the dynamic-field-from-nested-object branch from c4e0af2 to 256a8a4 Compare November 17, 2025 02:51
As these require the objects to be instantiated, this will break hooks if this is done before AfterHooks. We require it to be done after FirstWorldInit because some skills require certain global things e.g. MapGen to exist for their `create` function to work.
@LordMidas LordMidas force-pushed the dynamic-field-from-nested-object branch from 256a8a4 to 728309e Compare November 17, 2025 02:51
- By default we return the first match for the filename in the order the IO.enumerateFiles enumerates files.
- Subfolder path can still be specified for a specific match.
- Instead we try to find the item manually by iterating over all possible itemOwners until we match the itemId.
- If not found then we return the default ItemObjectByFilename.
This ensures that skills related to items use the item's properties e.g. damage instead of showing how the skill behaves for the actor himself.
@LordMidas
Copy link
Member Author

Closed in favor of #17

@LordMidas LordMidas closed this Nov 20, 2025
@LordMidas LordMidas deleted the dynamic-field-from-nested-object branch November 20, 2025 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dynamically generated text from target object of hyperlink

3 participants