Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Addon "Better Check" created #2049

Open
wants to merge 973 commits into
base: dev
Choose a base branch
from
Open

Addon "Better Check" created #2049

wants to merge 973 commits into from

Conversation

Sephodious
Copy link

Created an Addon called Better Check (bcheck) that livens up the check a bit. I know some other mods did this already but i didn't want all the extra that came with it. Just wanted to change the /check and nothing else.

RubenatorX and others added 30 commits September 10, 2020 13:32
Square removed one byte from end of character name (max character name length is 15 anyway so, not sure why it was ever 16, but there you go)
Updated for new battle message when the boss uses Imperfect Defense shell, this will require an update of resources before pushing to live.
Report file load error instead of erroneously saying "can't find the file".
Also brought default "binds" file into the loop
[battlmod] Update new ambu message
Updated to use the packets lib and cleared some gearswap code that was left on the addon.
[translate] Update to use the packets lib
fix for inventories that contains a space on their names.
Made this change so list command shows TVR songs.
[fields.lua] Wardrobes and Safe2 fixes
Added mini mode.
Added collectables toggle.
Fixed a critical bug that could end with a windower crash, added condensation for targets with the same name, several minor fixes.
New song October update
[battlemod] Critical fix and new feature added
…results

[battlemod] add crafting display toggle
Back to hardcoded packet modification instead of library, bug fix when several incoming text with mode 204 comes in a short period of time.
[translate] back to hardcoded packet, bug fix
Song name was confirmed from obtaining and using the key item "Sheet of harvest tunes" during the halloween 2020 event.
Copy link
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.

In addition to the comments within the code, the style of the code is a bit over the place and does not match our common style very well. Especially the indentation is off in some places, try to use 4 space indentations for every level of nesting. You have some parentheses that are unnecessary, some unnecessary spaces before function calls, and missing spaces after commas.

Aside from the indentation we don't require any of the other stylistic changes, but we'd still appreciate it to make our code base be uniform and easier to read.


-- sets local tables
local valid_message_ids = S{170,171,172,173,174,175,176,177,178}
local mchal = {'Very Weak':color(259),'Inredibly Easy Prey':color(259),'Easy Prey':color(259),'A Decent Challenge':color(2),'Evenly Matched':color(53),'Tough':color(159),'Very Tough':color(124),'Incredibly Tough':color(167)}
Copy link
Member

Choose a reason for hiding this comment

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

I would split this table (and mstat as well) into multiple lines, just so the colors are more easily adjustable. This could also be made configurable via settings, so every user can define their own colors, but it's fine for the first version.


-- gets mob level
if lvl > 0x7FFFFFFF then
lvl = -1
Copy link
Member

Choose a reason for hiding this comment

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

I think using something like '?' would be better than -1, I feel that would just be confusing for non-technical people who don't know that -1 is sometimes used as a dummy value.

Copy link
Collaborator

Choose a reason for hiding this comment

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

-1 is in line with the behavior of battlemod. The value is used to describe the mobs one would fight at level 1.

Copy link
Member

Choose a reason for hiding this comment

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

Still feels wrong unless -1 is the actual value reported. Speaking of which, what is the actual value?

Copy link
Collaborator

@RubenatorX RubenatorX Jul 13, 2021

Choose a reason for hiding this comment

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

2147483649 (0x80000001)

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh sorry nvm its reporting 4294967295 (0xFFFFFFFF) which is twos compliment for -1

Copy link
Collaborator

@RubenatorX RubenatorX Jul 13, 2021

Choose a reason for hiding this comment

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

okay so Sephodious, we talked it over and if you require('bit') with the rest of your requires... and then instead of doing...

            local lvl = mobinfo['Param 1']
            
            -- gets mob level
            if lvl > 0x7FFFFFFF then
                lvl = -1
            end

Just do

local lvl = bit.tobit(mobinfo['Param 1']) -- convert unsigned int to signed int

else if (message_id == 249) then
local target = windower.ffxi.get_mob_by_id(mobinfo['Target']) or {name=('Unknown')}

windower.add_to_chat(2, "[%s] Their power is over 9000!! (Impossible to gauge)":format(target.name:color(167)))
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if many people would appreciate that line 😅 Probably better to just use a neutral text, like Impossible to gauge.

Copy link
Contributor

Choose a reason for hiding this comment

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

I lol'd . IDK how I feel about this one. Arcon makes a point but these add-ons are made by the community and a little personal flavor text never hurt anyone. Its not like we're some real project.

-- uses incoming chunk so that we can block standard /check text
windower.register_event('incoming chunk',function (id,original,modified,injected,blocked)

-- ignores packets that don't begin with 29
Copy link
Member

Choose a reason for hiding this comment

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

Technically it ignores packets that don't have ID 0x029 :)


-- sets local tables
local valid_message_ids = S{170,171,172,173,174,175,176,177,178}
local mchal = {'Very Weak':color(259),'Inredibly Easy Prey':color(259),'Easy Prey':color(259),'A Decent Challenge':color(2),'Evenly Matched':color(53),'Tough':color(159),'Very Tough':color(124),'Incredibly Tough':color(167)}
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo on IEP - Inredibly should be Incredibly.
Is there a reason for changing "Too Weak to be Worthwhile" to "Very Weak"? I understand that some of the other slight changes are probably to allow for a more natural flowing sentence, but Too Weak... fits fine as it is.

Copy link
Contributor

Choose a reason for hiding this comment

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

Needs to match in-game terms imo.

local chal = mchal[mobinfo['Param 2'] - 63]
local stat = mstat[message_id - 170]

windower.add_to_chat(5, "The [%s] is (Lvl.%s), has ~%s~ and seems like its {%s}":format(target.name:color(2), tostring(lvl):color(213), stat:color(1), chal))
Copy link
Contributor

Choose a reason for hiding this comment

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

"seems like its" -> "seems like it's"

@Sephodious
Copy link
Author

Hey, sorry i haven't been around. I'll get started on all this later today or tomorrow and resubmit. Thanks for the suggestions and comments!

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.