Cutscene and Dialog System 0.0.5

[Nyctaeus]

This is when the history of the bravest Legionnaire of the Enclave begins...

The thing works as fully-fledged replacement for obsolete dialogue system of WC3, is cool, is full of QoL, is idiot-proof and idiot-friendly. Tested on myself.

 

[Macielos]

BTW, I know 95% of the map size is music, but I couldn't help myself to build a proper vibe
.

 

[Macielos]

Damn, I should have advertised it as a story of drunken footman first, and a dialogue system second xD.

 

[Antares]

This looks great. Do you think there's a way to fade in letters as they appear? Like in WoW quest texts?

 

[Macielos]

This looks great. Do you think there's a way to fade in letters as they appear? Like in WoW quest texts?

A whole text, yes, single letters - problematic (using transparency in text tags doesn't work as far as I know). So far I'm focused on functional improvements, making things pretty is not my specialty .

 

[Antares]

Hey,

regarding the fading in of characters, I wrote this snippet as a proof of concept. Maybe you'd like to make something out of it. I really think it would make your system look great.

Hidden

if Debug then Debug.beginFile "WowQuestText" end
do
    local TEXT_WIDTH = 0.2
    local CHARACTERS_PER_SECOND = 25
    local FADE_IN_TIME = 0.6
    local TEXT_LINE_SPACING = 0.01
    local ALPHA_INCREMENT = 5

    local X_LEFT = 0.3
    local Y_TOP = 0.5

    local widthTestFrame = nil              ---@type framehandle

    function WriteQuestText(text)
        local charNum = 0
        local thisWord = text:sub(1, text:find(" ") - 1)
        local length = text:len()
        local remainingText
        local char
        local x, y = X_LEFT, Y_TOP

        TimerStart(CreateTimer(), 1/CHARACTERS_PER_SECOND, true, function()
            ::begin::
            charNum = charNum + 1

            if charNum > length then
                DestroyTimer(GetExpiredTimer())
                return
            end
            char = text:sub(charNum, charNum)

            if char == "\n" then
                x = X_LEFT
                y = y - TEXT_LINE_SPACING
                goto begin
            elseif char == " " then --Check if next word fits into current line
                remainingText = text:sub(charNum + 1, length)
                thisWord = remainingText:sub(1, (remainingText:find(" ") or remainingText:len() + 1) - 1) --Find next space or is last word.
                BlzFrameSetText(widthTestFrame, " " .. thisWord:gsub("\n", "")) --Width must include previous space.
                BlzFrameSetSize(widthTestFrame, 0, TEXT_LINE_SPACING)
                if x + BlzFrameGetWidth(widthTestFrame) > X_LEFT + TEXT_WIDTH then --Next line
                    x = X_LEFT
                    y = y - TEXT_LINE_SPACING
                    goto begin
                end
            end

            local newFrame = BlzCreateFrameByType("TEXT", "textFrame", BlzGetOriginFrame(ORIGIN_FRAME_WORLD_FRAME, 0), "", 0)
            BlzFrameSetAbsPoint(newFrame, FRAMEPOINT_BOTTOMLEFT, x, y)
            BlzFrameSetText(newFrame, char)
            BlzFrameSetSize(newFrame, 0, TEXT_LINE_SPACING)
            local alpha = 0
            BlzFrameSetAlpha(newFrame, 0)
            TimerStart(CreateTimer(), ALPHA_INCREMENT*FADE_IN_TIME/255, true, function()
                alpha = alpha + ALPHA_INCREMENT
                BlzFrameSetAlpha(newFrame, alpha)
                if alpha >= 255 then
                    DestroyTimer(GetExpiredTimer())
                end
            end)
            x = x + BlzFrameGetWidth(newFrame)
        end)
    end

    OnInit.final(function()
        widthTestFrame = BlzCreateFrameByType("TEXT", "textFrame", BlzGetOriginFrame(ORIGIN_FRAME_WORLD_FRAME, 0), "", 0)
        WriteQuestText([[There was a time when I was young and full of vigor like you, druid.

But now I'm old and unable to explore the world like I once did. Ah, the things I saw!

But there was one mystery that eluded me. For my final quest, the Explorers' League sent me in search of The Temple of Atal'Hakkar. It was rumored to be located in the Swamp of Sorrows.

Because of my frail state, I traveled the skies by gryphon in search of it but never found it.

Help an old dwarf out? Perhaps you will have better luck on foot.]])
    end)
end

Moving on to the review:

The documentation of your API could be better. You're mixing configurables with non-configurables and constants with no clear delimitation. Why is "onSceneEndTrigger" in the config, for example? As far as I can tell, that just seems to be an upvalue declaration for later.

You should also try to abide by the formatting conventions laid out here (globals UpperCase, locals lowerCase, constants CAPS_LOCK).

A global simply named "utils" could lead to name clashes. Consider renaming it or try to find a way to not make it a global. One way to do it is use Lua's _ENV feature. You also randomly have functions not in the utils table in that script file. It's a bit of a mess.

Because you have no enclosing scope around your script files, the local printDebug functions are visible everywhere.

On re-entering the rifleman beacon, the background frame of the dialog is transparent. That seems not intended.

My first instinct when the first multiple choice dialog popped up was to press the number keys to select the option. I think that would be a great addition. Also consider making the hotkeys customizable.

All in all, this is a fantastic system and easily a high quality rating once all the rough edges have been smoothed out. Looking forward to using this myself .

Awaiting Update

 

[Macielos]

Thanks for feedback, I'll try to apply some fixes and refactors once I have a free moment.

The whole configuration is in ScreenplayVariants.lua, onSceneEndTrigger is just an internal variable. Perhaps you were misled by an old comment ("start of config settings") I forgot to remove once I extracted configuration from this class . I'll remove it.

Yeah, I admit I was a bit inconsistent with naming. Some were leftovers from the original library I was reworking, some (e.g. utils class) were left due to compatibility reasons, i.e. I have references to them in existing maps. Maybe I'll make proxy classes from them like I did with ScreenplayLegacyInterface.

As for keys, I used arrows mostly because simplicity and out of the box support in triggers without any additional library.

 

[Macielos]

Okay, I uploaded a bunch of fixes, refactors and some small new features. I only couldn't reproduce this case with invisible frame when re-visiting the dwarves - I remember I had it in the past, but now, nothing.

 

[Antares]

A lot of improvements, nice. The invisible frame bug doesn't appear anymore.

I see a small issue with the text as it is being written that the word first appears in the old line, then jumps to a new one once it no longer fits. I fixed that in the text system I mentioned last time by checking if the next word would fit in the current line first before starting to render it. If you wanna copy that solution, check it out here.

You have superfluous wrappers around your functions because some parts use the old function names? Can't you just go through your code with find and replace?

I don't see a mention in your description about whether your system is working in multiplayer. I assume it is not?

You're overwriting InitBlizzard for your initialization. If you're overwriting a native, you have to declare that in the documentation. While it's not required, I encourage you to use TotalInitialization as your initialization method. This ensures that your library works well with other libraries (if someone else is initializing their libraries the same way, only one will work (edit: nvm, I'm mistaken)).

While the documentation of the ScreenplayVariants is good, the ScreenplaySystem library misses a listing of the API, preferably at the top of the script. Also, are any of these function supposed to be private?

Because of the last two issues, gonna have to put it back to awaiting update.

 

[Macielos]

Sorry, I haven't had much time to work on WC3 scripts recently. I added API list (not complete one, but with functions people will most likely use) and some more docs, including info about InitBlizzard and multi.

I see a small issue with the text as it is being written that the word first appears in the old line, then jumps to a new one once it no longer fits. I fixed that in the text system I mentioned last time by checking if the next word would fit in the current line first before starting to render it. If you wanna copy that solution, check it out here.

Thanks, I might use it once I get to beautification stage .

You have superfluous wrappers around your functions because some parts use the old function names? Can't you just go through your code with find and replace?

I have replaced them in code long ago, but they are still used in multiple maps too. The libraries are in a single folder that I always copy between maps as a whole, but screenplays are kept near specific triggers, so I'd have to manually copy them file by file (unless there's some automagical hot-reload tool I am not aware of). So I did this workaround so that they work for now, and I will copy them lazily only when I make some changes in them. Believe me of not, this is a big time optimization .

This ensures that your library works well with other libraries (if someone else is initializing their libraries the same way, only one will work (edit: nvm, I'm mistaken)).

I think they will work no matter the order, each one will override the previous one.

While the documentation of the ScreenplayVariants is good, the ScreenplaySystem library misses a listing of the API, preferably at the top of the script. Also, are any of these function supposed to be private?

Well, some of these functions could indeed be private and they would be in a "normal" language. However, in my maps (where i have this lib + many others) I run into a limitation of max. 200 local functions, so I have to limit their usage. Besides, as far as I noticed, these local functions are not really local since they're compiled into one big script and their accessibility just depends on script order. Plus - that's my personal bias of course - I don't like the fact i need to declare a function higher than its invocation - it reduces code readibility.

 

[Antares]

I think they will work no matter the order, each one will override the previous one.

You're right. I figured that out after I posted my comment.

Well, some of these functions could indeed be private and they would be in a "normal" language. However, in my maps (where i have this lib + many others) I run into a limitation of max. 200 local functions, so I have to limit their usage. Besides, as far as I noticed, these local functions are not really local since they're compiled into one big script and their accessibility just depends on script order. Plus - that's my personal bias of course - I don't like the fact i need to declare a function higher than its invocation - it reduces code readibility.

I was talking about the fact that you have no API listing and you might have functions in your Screenplay table that are to be considered private, adding to the confusion.

I've run into the 200 locals limit as well while working on ALICE, but that is a much larger script than what you have here, so that's surprising to me. I also don't understand what you're saying about local functions are not local. Are you putting a scope around them with do end?

If you run into the problem of no-forward declaration, you can define the function variable as nil at the top of the scope, then overwrite it further down. Example:

do
    local testFunc = nil
    
    local function CallTestFunc()
        testFunc()
    end

    testFunc = function() print("test succesful") end

    CallTestFunc()
end

 

[Macielos]

I was talking about the fact that you have no API listing and you might have functions in your Screenplay table that are to be considered private, adding to the confusion.

Okay, sorry for a long delay, I uploaded some refactors. I added scopes and made funcs local where possible, now that they are truly local. I also got rid of the remaining global funcs completely.

I mean, I know how to do a forward declaration, I just don't like it as it reduces code readability. In a OOP language you usually have public methods on top, which then call some private methods, which are below, and you read it like a book. Here I have to either jump up and down, OR have this forward declarations. It's not a blocker, of course, it's just annoying. But I guess I need to get rid of some biases from Java-like languages when coding in LUA.

I think in my next libs I'll limit usage of LUA classes and make them stateless, and keep all internal variables as local scoped. Right now fields of e.g. ScreenplaySystem are in fact public, but they needn't be.

I've run into the 200 locals limit as well while working on ALICE, but that is a much larger script than what you have here, so that's surprising to me.

I ran into this limit on another map, where I have this library plus several others, including e.g. lots of custom skills. I need to check out if putting local funcs within scopes make them not count to this limit.

 

[Antares]

Good progress on the code refactoring. It looks a lot cleaner! However

onInit(function()
    ScreenplaySystem:init()
end)

A three line library? Can't this be part of the ScreenplaySystem script?

ScreenplaySystem = {   -- main dialogue class, configuration options can be found in ScreenplayVariants.lua
    FDF_BACKDROP = "EscMenuBackdrop",
    FDF_TITLE = "CustomText", -- from imported .fdf
    FDF_TEXT_AREA = "CustomTextArea", -- ``
    TITLE_COLOR_HEX = "|cffffce22", -- character title text color.
    TEXT_COLOR_HEX = "|cffffffff", -- character speech text color.
    INACTIVE_CHOICE_COLOR_HEX = "|cff808080", -- greyish text color for choices other than the selected one

    debug = false, -- print debug messages for certain functions.

    --leftovers from original lib, could be moved to ScreenplayVariants, but so far I saw no need to customize them
    fade = true, -- should dialogue components have fading eye candy effects?
    fadeDuration = 0.81, -- how fast to fade if fade is enabled.

    --internal state of the config and the screenplay
    frameInitialized = false,
    currentVariantConfig = nil,
    onSceneEndTrigger = nil,
    messageUncoverTimer,
    trackingCameraTimer,
    autoplayTimer,
    cameraInterpolationTimer,
    delayTimer,
    fadeoutTimer,
    lastActorUnitTypeSpeaking,
}

You're not defining the fields messageUncoverTimer, trackingCameraTimer etc. here, but assigning undeclared globals to the sequence part of the table. This makes my VS Code go crazy with warnings. You need to add the = nil to each of them.

function SimpleUtils.debugFunc(func, name)
    local name = name or ""
    local result
    local passed, data = pcall(function()
        result = func()
        return "func " .. name .. " passed"
    end)
    if not passed then
        print(name, passed, data)
    end
    passed = nil
    data = nil
    return result
end

You're generating a lot of anonymous functions throughout your code. You could try to reduce that on those that are called more often, like the debugFunc here.

I have the feeling that the ScreenplayFactory:saveBuilder function could be solved in a much simpler way with an optional name parameter in the builder functions.

The inconsistent use of dot and colon syntax is also something you should address. This can lead to a lot of headache for a user when using the wrong notation. For functions such as function ScreenplaySystem:init(), you could simply remove the colon and set local self = ScreenplaySystem in the first line of the function. Adding type notations with EmmyLUA would also help immensely.

Apart from these issues, I'm still missing an overarching documentation. You have a lot of different sub-libraries and it is not obvious what the exact responsibilities of each of them are to the uninitiated user. You have different snippets of information scattered throughout the hiveworkshop page, the test map triggers, and your code, but nothing that I can just read and follow to get started. I want to have a tutorial for "I have imported the libraries into my map. Now what?" It should guide me through step by step how I can create my first screenplay, list the API, and explain what the responsibilities of each library are.

A good documentation is really crucial for a resource this complex!