Sign in to follow this  

GetOfficialCharacterList returning too many indexes


CarlZalph
  • Version: PC/MAC/Linux Pending

In: data/scripts/dlcsupport.lua:

function GetOfficialCharacterList()
    local list = MAIN_CHARACTERLIST

    if IsDLCEnabled(REIGN_OF_GIANTS) then
        list = JoinArrays(list, ROG_CHARACTERLIST)
    end
    if IsDLCEnabled(CAPY_DLC) then
        if IsDLCInstalled(REIGN_OF_GIANTS) then
            list = JoinArrays(list, ROG_CHARACTERLIST)
        end
        list = JoinArrays(list, SHIPWRECKED_CHARACTERLIST)
    end
    if IsDLCEnabled(PORKLAND_DLC) then
        if IsDLCInstalled(REIGN_OF_GIANTS) then
            list = JoinArrays(list, ROG_CHARACTERLIST)
        end
        if IsDLCInstalled(CAPY_DLC) then
            list = JoinArrays(list, SHIPWRECKED_CHARACTERLIST)
        end          
        list = JoinArrays(list, PORKLAND_CHARACTERLIST)
    end    
    return list
end

This function is repeatedly joining arrays to the return value multiple times.

Output:

Spoiler

1  wilson
2  willow
3  wolfgang
4  wendy
5  wx78
6  wickerbottom
7  woodie
8  wes
9  waxwell
10 wathgrithr
11 webber
12 wathgrithr
13 webber
14 walani
15 warly
16 wilbur
17 woodlegs
18 wathgrithr
19 webber
20 walani
21 warly
22 wilbur
23 woodlegs
24 warbucks
25 wilba

 

Note the repeats of the DLC characters.

 

 

To fix, the function should be:

function GetOfficialCharacterList()
    local list = MAIN_CHARACTERLIST

    if IsDLCEnabled(REIGN_OF_GIANTS) then
        list = JoinArrays(list, ROG_CHARACTERLIST)
    end
    if IsDLCEnabled(CAPY_DLC) then
        list = JoinArrays(list, SHIPWRECKED_CHARACTERLIST)
    end
    if IsDLCEnabled(PORKLAND_DLC) then   
        list = JoinArrays(list, PORKLAND_CHARACTERLIST)
    end    
    return list
end

 

 

Side note the function 'GetActiveCharacterListForSelection' right below this one should be calling this function for its initial list generation- it's copy paste code and both need to be in sync.

Example:

Spoiler

function GetActiveCharacterListForSelection()
    local list = GetOfficialCharacterList()

    for i=#list,1,-1 do
        for t,rchar in ipairs(RETIRED_CHARACTERLIST)do
            if rchar == list[i] then
                table.remove(list,i)
            end
        end
    end        

    return JoinArrays(list, MODCHARACTERLIST)
end

 

 


Steps to Reproduce
Read the files and/or use the functions. They're not right.

Status: Pending

This issue has not been confirmed by a developer yet.


  • Thanks 1
  Report Bug
Sign in to follow this  


User Feedback


....

Hmm, if I understand the intent of the original code correctly, only one of the top-level if-clauses is supposed to be executed, but the list you received indicates that they all were.

My suggestion to retain the original intention would be the following:

function GetOfficialCharacterList()
    local list = MAIN_CHARACTERLIST

    if IsDLCEnabled(PORKLAND_DLC) then
        if IsDLCInstalled(REIGN_OF_GIANTS) then
            list = JoinArrays(list, ROG_CHARACTERLIST)
        end
        if IsDLCInstalled(CAPY_DLC) then
            list = JoinArrays(list, SHIPWRECKED_CHARACTERLIST)
        end          
        list = JoinArrays(list, PORKLAND_CHARACTERLIST)
    elseif IsDLCEnabled(CAPY_DLC) then
        if IsDLCInstalled(REIGN_OF_GIANTS) then
            list = JoinArrays(list, ROG_CHARACTERLIST)
        end
        list = JoinArrays(list, SHIPWRECKED_CHARACTERLIST)
    elseif IsDLCEnabled(REIGN_OF_GIANTS) then
        list = JoinArrays(list, ROG_CHARACTERLIST)
    end    
    return list
end

Checking for enabled DLC in order from latest to earliest and using elseif to ensure only one is executed should work as intended.

Edited by JustTheBast
Styling fix

Share this comment


Link to comment
Share on other sites
....
5 hours ago, JustTheBast said:

Hmm, if I understand the intent of the original code correctly, only one of the top-level if-clauses is supposed to be executed, but the list you received indicates that they all were.

My suggestion to retain the original intention would be the following:


function GetOfficialCharacterList()
    local list = MAIN_CHARACTERLIST

    if IsDLCEnabled(PORKLAND_DLC) then
        if IsDLCInstalled(REIGN_OF_GIANTS) then
            list = JoinArrays(list, ROG_CHARACTERLIST)
        end
        if IsDLCInstalled(CAPY_DLC) then
            list = JoinArrays(list, SHIPWRECKED_CHARACTERLIST)
        end          
        list = JoinArrays(list, PORKLAND_CHARACTERLIST)
    elseif IsDLCEnabled(CAPY_DLC) then
        if IsDLCInstalled(REIGN_OF_GIANTS) then
            list = JoinArrays(list, ROG_CHARACTERLIST)
        end
        list = JoinArrays(list, SHIPWRECKED_CHARACTERLIST)
    elseif IsDLCEnabled(REIGN_OF_GIANTS) then
        list = JoinArrays(list, ROG_CHARACTERLIST)
    end    
    return list
end

Checking for enabled DLC in order from latest to earliest and using elseif to ensure only one is executed should work as intended.

Doing that is functionally equivalent to the 'fixed' code posted, and looks as much of a jumble as the 'unfixed'.

Share this comment


Link to comment
Share on other sites
....
56 minutes ago, CarlZalph said:

Doing that is functionally equivalent to the 'fixed' code posted, and looks as much of a jumble as the 'unfixed'.

I don't believe it is.  Unless IsDLCEnabled(id) is identical to IsDLCInstalled(id), those code pieces are not equivalent.  My impression is that the code was supposed to check what type of world is selected (Base, RoG, SW, or Ham), and then make available all characters from DLC earlier than the selected, which also happen to be installed.  For example, a user who has RoG and Hamlet installed and is starting a Hamlet-only game would still get Wigfrid and Webber in the character list, even though RoG is not enabled — In your abbreviated version, they would not.

Admittedly, that's just what I think the original code was supposed to do, and what "enabled" and "installed" mean in this context.

Share this comment


Link to comment
Share on other sites
....

You are both correct , I think. I thin the outer ifs should be elseifs but I'll have to double check.

(the code should be doing a union on the arrays regardless I'd say)

  • Thanks 1

Share this comment


Link to comment
Share on other sites


Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now