Skip to content

Outfit lua metatable#3027

Merged
nekiro merged 2 commits intootland:masterfrom
nekiro:outfit-metatable
Oct 26, 2020
Merged

Outfit lua metatable#3027
nekiro merged 2 commits intootland:masterfrom
nekiro:outfit-metatable

Conversation

@nekiro
Copy link
Member

@nekiro nekiro commented May 5, 2020

This pr adds Outfit() metatable to use in lua. Helpful in cases when you for example need to get name of outfit from looktype.

Outfit(looktype)
and outfit compare method

@nekiro nekiro added the enhancement Increase or improvement in quality, value, or extent label May 5, 2020
@nekiro nekiro requested a review from a team October 21, 2020 10:46
Copy link
Member

@Znote Znote left a comment

Choose a reason for hiding this comment

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

Beautiful and educational code that works great as a sample on how to metatablify something. :)

@nekiro nekiro merged commit 36ca28b into otland:master Oct 26, 2020
@nekiro nekiro deleted the outfit-metatable branch October 26, 2020 13:51
@nekiro
Copy link
Member Author

nekiro commented Oct 26, 2020

Thanks @marmichalski, not sure why this pr had so much of weird code, but it was pretty old too.

nekiro added a commit that referenced this pull request Oct 26, 2020
@ranisalt
Copy link
Member

Beautiful and educational code that works great as a sample on how to metatablify something. :)

I think @nekiro 's thought process deserves an entry in the wiki ;)

@nekiro
Copy link
Member Author

nekiro commented Oct 28, 2020

Beautiful and educational code that works great as a sample on how to metatablify something. :)

I think @nekiro 's thought process deserves an entry in the wiki ;)

Are you rude???

@Znote
Copy link
Member

Znote commented Oct 31, 2020

This did not work as good as I was hoping for, got time to test it out today:

local talkaction = TalkAction("!hello")

function talkaction.onSay(player)
   print(player:getName() .. ' executed talkaction: !hello')
   
   tdump("player:getOutfit()", player:getOutfit())
   tdump("Outfit(140)", Outfit(140))
   tdump("Outfit(140) == Outfit(140) -- Noblewoman == Noblewoman", Outfit(140) == Outfit(140))
   tdump("Outfit(140) == player:getOutfit() -- Noblewoman == Noblewoman", Outfit(140) == player:getOutfit())
   tdump("Outfit(140) == Outfit(151) -- Noblewoman == Pirate", Outfit(140) == Outfit(151))
   tdump("Outfit(140) == Outfit(132) -- Noblewoman == Nobleman", Outfit(140) == Outfit(132))

   return true
end

talkaction:register()

I think there should be a bit more consistency between player:getOutfit and Outfit, and I think it would be great if they could compare against each other, and make it possible to match outfits across gender/sex.

image

player:getOutfit() is unable to compare against Outfit(), even when looktype matches.
player:getOutfit() is missing data such as premium, unlocked, name

Is there no way to compare "matching set across gender"? I want Outfit(132) == Outfit(140) to match true as in "I want to see if this is noble outfit" regardless of gender (and thus looktype).

I find it a bit redundant to first get player outfit, then initialize a new outfit object before I can compare it.

local noblewoman = Outfit(140)
if noblewoman ~= Outfit(player:getOutfit().lookType) then
    -- Only fancy rich women allowed!
end

It would also be cool to initialize an Outfit by the name.

-- Sexy code
if player:getOutfit() == Outfit("Noblewoman") then
    -- ...
end

@ranisalt
Copy link
Member

ranisalt commented Oct 31, 2020

@Znote but an outfit is more than just the looktype, there are colors, addons, gender and more as you shown. How do you want two outfits to equal when the only value that is equal is the looktype? It should read if player:getOutfit().lookType == 140 then if you want to check if that's the noble[wo]man outfit.

It equals when you do Outfit(140) == Outfit(140) because all attributes are equal, not just looktype.

Edit: ah, now I see the problem with the outfit gender. I don't think there's an easy way to fix it, since there's no "gender" to outfits, there are two instances of each.

@Znote
Copy link
Member

Znote commented Oct 31, 2020

I argue that since the object is created from lookType, it should be able to match against player:getOutfit(), it doesn't matter which colours you dress, your either wearing noblewomen or your not. I would rather take an extra step in the code if the color mattered, instead of vice versa.

But even this:

if player:getOutfit().lookType == Outfit("Noblwomen").lookType then
    -- ...
end

Would be a nice improvement, I doubt everyone remembers what lookType 140 is, but if its initialized as "Noblewomen" (or OUTFIT_NOBLEWOMEN) if we wanna use enums, then its pretty easy to understand what outfit we are working with, what outfit we want the player to have.

Yeah I figured the gender match was a stretch, the XML file at least has to be structured differently to accomodate gender groupings of outfits.

@marmichalski
Copy link
Contributor

@Znote you're mixing something I'd call PlayerOutfit with XmlOutfit.

@Kamenuvol
Copy link
Contributor

Maybe call OutfitType metatable instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Increase or improvement in quality, value, or extent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants