[Need Opinion] Making Mod-Characters locked by default

[Malacath]

So I've been doing this for the first time for the Waverly mod and I had a quite queasy feeling back then because I had to mess with the PlayerProfile. Now I'm getting in the position where I have to do it again and ideally I would like the two mods to be compatible and adjusted my code accordingly

function ModPlayerProfile(PlayerProfile)    local oldIsCharacterUnlocked = PlayerProfile.IsCharacterUnlocked    function PlayerProfile:IsCharacterUnlocked(character)        local oldres = oldIsCharacterUnlocked(self, character)        if character == "waverly" and not self.persistdata.unlocked_characters[character] then            return false        end        return oldres    endendAddGlobalClassPostConstruct("playerprofile", "PlayerProfile", ModPlayerProfile)

Assuming all mods would do it like this I think that it would work just fine but I'd really like an opinion of a good modder, or maybe two... hell the opinion of any modder coming by would be a great thing.

Main reason I'm asking this is because I'm just not feeling to confident messing with things like the PlayerProfile

 

So thanks to everyone considering to help me here  ^^

[Nycidian]

So I've been doing this for the first time for the Waverly mod and I had a quite queasy feeling back then because I had to mess with the PlayerProfile. Now I'm getting in the position where I have to do it again and ideally I would like the two mods to be compatible and adjusted my code accordingly

function ModPlayerProfile(PlayerProfile)    local oldIsCharacterUnlocked = PlayerProfile.IsCharacterUnlocked    function PlayerProfile:IsCharacterUnlocked(character)        local oldres = oldIsCharacterUnlocked(self, character)        if character == "waverly" and not self.persistdata.unlocked_characters[character] then            return false        end        return oldres    endendAddGlobalClassPostConstruct("playerprofile", "PlayerProfile", ModPlayerProfile)

Assuming all mods would do it like this I think that it would work just fine but I'd really like an opinion of a good modder, or maybe two... hell the opinion of any modder coming by would be a great thing.

Main reason I'm asking this is because I'm just not feeling to confident messing with things like the PlayerProfile

 

So thanks to everyone considering to help me here  ^^

 

Ok silly question why not just add waverly to persistdata.unlocked_characters?

 

From the regular function it looks like all that would need to happen for the function to work like you want is for your character to be in that function  table

function PlayerProfile:IsCharacterUnlocked(character)    if character == "wilson" then		return true    end        if self.persistdata.unlocked_characters[character] then        return true	end	if not table.contains(CHARACTERLIST, character) then		return true -- mod character	end	return falseend

Edit: To clarify, you would need to add your character to CHARACTERLIST by default and then add it to persistdata.unlocked_characters when it became unlocked.

[squeek]

Ok silly question why not just add waverly to persistdata.unlocked_characters?

 

From the regular function it looks like all that would need to happen for the function to work like you want is for your character to be in that function

It being in the persistdata.unlocked_characters table would make it unlocked, not locked, and all mod characters are unlocked by default anyway (even if they're not in the unlocked_characters table) because of the third if check in the IsCharacterUnlocked() function.

Malacath, looks like a perfectly acceptable method to me. If all mods used the same method, there would not be any conflicts.

EDIT: Judging by the code Nycidian posted, you could add your custom character to the global CHARACTERLIST table to make it respect the character not being in the unlocked_characters table, but I'm not sure what side-effects that may have. I think the method in the OP is probably the cleanest.

[Heavenfall]

Yeah, looks fine to me. That's how I try to do my core function overwrites as long as they are returning something.

 

Well, except my mod to unlock all characters that happens to overwrite the same function:

function HF_unlockallreleasedchars(self)    local playerprofileclasshook = GLOBAL.PlayerProfile    playerprofileclasshook.IsCharacterUnlocked = function()            return true    endendAddGamePostInit(HF_unlockallreleasedchars)

but the point of that is kind of to unlock everything always.

[Nycidian]

It being in the persistdata.unlocked_characters table would make it unlocked, not locked, and all mod characters are unlocked by default anyway (even if they're not in the unlocked_characters table) because of the third if check in the IsCharacterUnlocked() function.

Malacath, looks like a perfectly acceptable method to me. If all mods used the same method, there would not be any conflicts.

 

As i edited while you were replying

 

To clarify, you would need to add your character to CHARACTERLIST by default and then add it to persistdata.unlocked_characters when it became unlocked.

 

 

I'm pretty sure that would work without touching the function and would always be compatable with other mods as long as you just added to the tables and did not overwrite characters. Am I wrong?

[Malacath]

Malacath, looks like a perfectly acceptable method to me. If all mods used the same method, there would not be any conflicts.

Thanks, I value your opinion so hearing that is great.

Yeah, looks fine to me. That's how I try to do my core function overwrites as long as they are returning something.

 

Well, except my mod to unlock all characters that happens to overwrite the same function:

but the point of that is kind of to unlock everything always.

function HF_unlockallreleasedchars(self)    local playerprofileclasshook = GLOBAL.PlayerProfile    playerprofileclasshook.IsCharacterUnlocked = function()            return true    endendAddGamePostInit(HF_unlockallreleasedchars)

Guess where I learned doing it like this  ; )

And also thanks for agreeing with Sqeek

To clarify, you would need to add your character to CHARACTERLIST by default and then add it to persistdata.unlocked_characters when it became unlocked.

 

 

I'm pretty sure that would work without touching the function and would always be compatable with other mods as long as you just added to the tables and did not overwrite characters. Am I wrong?

This sounds like a cleaner idea to me, I will definately try that and see/try to find out if it does any harm.

[Nycidian]

EDIT: Judging by the code Nycidian posted, you could add your custom character to the global CHARACTERLIST table to make it respect the character not being in the unlocked_characters table, but I'm not sure what side-effects that may have. I think the method in the OP is probably the cleanest.

I looked at the variable CHARACTERLIST all it is is a simple array with the character prefab names so my guess is nothing bad would happen doing simply

 

table.insert( CHARACTERLIST, "waverly")

[Malacath]

Look at the variable CHARACTERLIST all it is is a simple array with the character prefab names so my guess is nothing bad would happen doing simply

 

table.insert( CHARACTERLIST, "waverly")

Sure it's just a table but I don't know where it's used and if there would be any consequences coming from that.

[squeek]

As i edited while you were replying

 

To clarify, you would need to add your character to CHARACTERLIST by default and then add it to persistdata.unlocked_characters when it became unlocked.

 

 

I'm pretty sure that would work without touching the function and would always be compatable with other mods as long as you just added to the tables and did not overwrite characters. Am I wrong?

And I was editing while you were replying!

The game uses a separate global table for mod-created characters called MODCHARACTERLIST, so adding your mod character to CHARACTERLIST actually could have some side-effects (and even if it doesn't now, it has the potential to in the future).

EDIT: This topic is moving too quickly!

[Heavenfall]

Adding it to CHARACTERLIST is how we did it in the way back when, I moved away from it because there are functions in the game that cycle through it. Actually I'm pretty sure IPSquiggle added AddModCharacter() just because of it. Anyway, recommend you don't just add to CHARACTERLIST. And why would you? You've got a perfectly good hook into IsCharacterUnlocked() and you've got the persist data save, so what's the problem...

[Malacath]

Okay, it's two against one all opinions I value. So I will go with the solution I have that worked until now.

 

Thank you all for the good and fast responses ^^  I'm feeling way more confident about his now!

[simplex]

@Malacath

I also think your method is the ideal one.

[Malacath]

More opinion to value  ^^  What a great day.

[Nycidian]

What about this as a more universal solution/

function ModPlayerProfile(PlayerProfile)    local oldIsCharacterUnlocked = PlayerProfile.IsCharacterUnlocked    function PlayerProfile:IsCharacterUnlocked(character)        local oldres = oldIsCharacterUnlocked(self, character)        if table.contains(LOCKEDMODCHARACTERLIST, character) and not self.persistdata.unlocked_characters[character] then            return false        end        return oldres    endendAddGlobalClassPostConstruct("playerprofile", "PlayerProfile", ModPlayerProfile)

Then add any character you want locked into LOCKEDMODCHARACTERLIST

 

And add any character you want to unlock to persistdata.unlocked_characters

 

You then can use the same solution for all your mod characters or if you add multiple in a single mod and others could use the same solution.

 

Just a thought.

[Malacath]

What about this as a more universal solution/

function ModPlayerProfile(PlayerProfile)    local oldIsCharacterUnlocked = PlayerProfile.IsCharacterUnlocked    function PlayerProfile:IsCharacterUnlocked(character)        local oldres = oldIsCharacterUnlocked(self, character)        if table.contains(LOCKEDMODCHARACTERLIST, character) and not self.persistdata.unlocked_characters[character] then            return false        end        return oldres    endendAddGlobalClassPostConstruct("playerprofile", "PlayerProfile", ModPlayerProfile)

Then add any character you want locked into LOCKEDMODCHARACTERLIST

 

And add any character you want to unlock to persistdata.unlocked_characters

 

You then can use the same solution for all your mod characters or if you add multiple in a single mod and others could use the same solution.

 

Just a thought.

I was trying exactly that before reverting to my old solution. I have no clue what stupid things I did but I got only this in log.txt

Forced aborting...

So I was really scared and left that idea alone.

 

EDIT: And it would also require that the basic mod which overrides the function is enabled which will make things more complicated

[Nycidian]

 

EDIT: And it would also require that the basic mod which overrides the function is enabled which will make things more complicated

 

With "the basic mod" are you talking about a secondary mod to add this functionality to your mods?

[Malacath]

With "the basic mod" are you talking about a secondary mod to add this functionality to your mods?

I just mean there has to be one mod which overrides IsCharacterUnlocked so that mod must be anabled no matter what. That would either mean everyone who wants to play with Waverly will also have to enable the additional mod or everyone who wants to play with, let's call him, Wilfred has to enable the Waverly mod whihc contains the override of IsCharacterUnlocked

[Nycidian]

I just mean there has to be one mod which overrides IsCharacterUnlocked so that mod must be anabled no matter what. That would either mean everyone who wants to play with Waverly will also have to enable the additional mod or everyone who wants to play with, let's call him, Wilfred has to enable the Waverly mod whihc contains the override of IsCharacterUnlocked

 

Actually there is a way to do that without overwriting you make a sub mod like I'm designing with AManager that only loads once no matter how many mods its in. 

 

AManager currently can be placed in 100 different mods and it would only be present once ingame and since this in't a component it would actually be far easier to do as I had issues with components but sqeek helped me figure it out.

[squeek]

Using a mod-created global variable actually is more prone to conflict because other mods could overwrite it (and in this case, it would be pretty easy for a mod to overwrite it if they're not careful).

To avoid the table getting reset from each mod, you'd have to make sure every mod does:

GLOBAL.LOCKEDMODCHARACTERLIST = GLOBAL.LOCKEDMODCHARACTERLIST or {}-- insert a charactertable.insert( GLOBAL.LOCKEDMODCHARACTERLIST, "waverly" )

when defining it. It's definitely better to avoid requiring every mod to play nice if you can help it.

Then you'd also have each mod adding duplicate code to PlayerProfile:IsCharacterUnlocked. If two mods had the same ClassPostConstruct, then the checks would basically end up looking like this:

 

function PlayerProfile:IsCharacterUnlocked( character )    -- base checks    -- first mods added check    if table.contains(LOCKEDMODCHARACTERLIST, character) and not self.persistdata.unlocked_characters[character] then        return false    end    -- second mods added check    if table.contains(LOCKEDMODCHARACTERLIST, character) and not self.persistdata.unlocked_characters[character] then        return false    endend

The method in the OP doesn't really have any drawbacks that I can see. To extend it to support multiple characters in one mod, all you'd have to do is change the conditional to:

 

if (character == "waverly" or character == "otherchar") and not self.persistdata.unlocked_characters[character] then

EDIT: You could use a local table if you wanted, though

 

local MyUnlockableModChars = { "waverly", "otherchar" }-- the new conditionalif table.contains( MyUnlockableModChars, character ) and not self.persistdata.unlocked_characters[character] then

[Malacath]

Actually there is a way to do that without overwriting you make a sub mod like I'm designing with AManager that only loads once no matter how many mods its in. 

 

AManager currently can be placed in 100 different mods and it would only be present once ingame and since this in't a component it would actually be far easier to do as I had issues with components but sqeek helped me figure it out.

Of course that would work but I don't feel that's worth if for 11 lines of code   ; )

[Nycidian]

Of course that would work but I don't feel that's worth if for 11 lines of code   ; )

Well yes you have a point I was just responding to your query about having compatibility between multiple mods.

[squeek]

Actually there is a way to do that without overwriting you make a sub mod like I'm designing with AManager that only loads once no matter how many mods its in. 

 

AManager currently can be placed in 100 different mods and it would only be present once ingame and since this in't a component it would actually be far easier to do as I had issues with components but sqeek helped me figure it out.

The method used for AManager really only works when you're dealing with mod(s) that you have total control of. When you're modifying things common to all mods, you should always attempt to make things as self-contained as possible so that they work regardless of how other mods try to do similar things.

[simplex]

The method used for AManager really only works when you're dealing with mod(s) that you have total control of. When you're modifying things common to all mods, you should always attempt to make things as self-contained as possible so that they work regardless of how other mods try to do similar things.

And that's why I designed wicker to be embedded and localized. ;P

(I really should find the time to write some doc and/or tutorials on it...)

But this is borderline relevant. On topic: I maintain my position that Malacath's originally posted method is the preferred one, minimizing risks of incompatibility and being overall clean.

[Nycidian]

This is off topic sorry malacath, though it seems your question was answered so I won't feel to bad about derailment.

 

 

The method used for AManager really only works when you're dealing with mod(s) that you have total control of. When you're modifying things common to all mods, you should always attempt to make things as self-contained as possible so that they work regardless of how other mods try to do similar things.

 

I'm asking this for my mod BTW. 

 

Do you you think a more secure way rather than using global variable would be push and listen for event calls to communicate between the sub mods? It would be a bit more complicated but less likely to be messed with by other mods I would think.

 

I just don't know of another way to communicate between mods other than events or the global variable.

[simplex]

This is off topic sorry malacath, though it seems your question was answered so I won't feel to bad about derailment.

 

 

 

I'm asking this for my mod BTW. 

 

Do you you think a more secure way rather than using global variable would be push and listen for event calls to communicate between the sub mods? It would be a bit more complicated but less likely to be messed with by other mods I would think.

 

I just don't know of another way to communicate between mods other than events or the global variable.

Why don't you just write it as an external mod, expose an API to be loaded by require() and let other mods optionally use it? This "one size fit" submod will only give you headaches, and in particular you'll have to keep the interface essentially frozen to avoid breaking backwards compatibility...