Skip to content

Records of eminence addon#1585

Merged
cairthenn merged 9 commits into
Windower:devfrom
cairthenn:dev
Oct 31, 2017
Merged

Records of eminence addon#1585
cairthenn merged 9 commits into
Windower:devfrom
cairthenn:dev

Conversation

@cairthenn
Copy link
Copy Markdown
Member

No description provided.

Copy link
Copy Markdown
Member

@z16 z16 left a comment

Choose a reason for hiding this comment

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

👍

Comment thread addons/roe/roe.lua

local function save(name)
if not type(name) == "string" then
error('`save` : specify a profile name')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Markdown doesn't actually work here ;D If that was the intent behind using ``.

Since you're using logger it will redirect error to the FFXI log, so you can use FFXI coloring functions to highlight terms.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think I've avoided color codes forever because they have the rare chance to bug out the chat and I couldn't really pinpoint why

Comment thread addons/roe/roe.lua

settings.profiles[name] = S(_roe.active:keyset())
settings:save('global')
notice('saved %d objectives to the profile %s':format(_roe.active:length(), name))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sometimes you start sentences with upper case and sometimes with lower case!

Comment thread addons/roe/roe.lua Outdated


local needed_quests = settings.profiles[name]:diff(_roe.active:keyset())
local available_slots = 30 - _roe.active:length()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Magic number! Is that a fixed constant? Maybe put it somewhere near the top for easy adjustment, if that can be ever adjusted.

I just saw below it was the length of the packet payload, so we could derive the number from the fields.lua file. That info should be available and exposed through the packets API.

Comment thread addons/roe/roe.lua Outdated
end

local true_strings = S{'true','t','y','yes','on'}
local false_strings = S{'false','f','n','off'}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

'no' is missing here, unless that was intentional.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It was not!

Comment thread addons/roe/roe.lua Outdated
local function inc_chunk_handler(id,data)
if id == 0x111 then
_roe.active:clear()
for i = 1,30 do
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The same 30 as above? I assume that's packet payload length, as indicated in fields.lua. Any reason not to use packet.parse here?

Copy link
Copy Markdown
Member Author

@cairthenn cairthenn Oct 31, 2017

Choose a reason for hiding this comment

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

The only reason was so I didn't have to put together a string with index i :( I hate strings :(

Comment thread addons/roe/roe.lua Outdated
return (k + 1024*data:unpack('H', 133) - 1)
end):map(
function(v)
return (v == 1 and true) or false
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This line should be equivalent to return v == 1, no?

Since they use this type of bit field quite a bit (quests, missions, key items, maps, etc.) we should maybe introduce a specific type for it so we don't need to do manual parsing...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Whoops. And yeah, I think it would be a good idea.

@cairthenn cairthenn merged commit 7774028 into Windower:dev Oct 31, 2017
z16 pushed a commit that referenced this pull request Nov 4, 2024
Records of eminence addon
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.

2 participants