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

New Script: gui/cult-lists #664

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

gero-pardo
Copy link

Info gathering script for deities and religions.
Comes with a GUI and print to console options.
It's all set code-wise, but most likely not up to lint and formatting standards
Somewhat informal with comments, I'll remove if requested.

Copy link
Member

@myk002 myk002 left a comment

Choose a reason for hiding this comment

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

could you attach a few screenshots of the UI to the PR description? If you take a screenshot, you can literally just edit the description and hit ctrl-v to paste the image in.


--TO-DO
-- zoom on unit -- can't be done yet
-- never touch LUA script again
Copy link
Member

Choose a reason for hiding this comment

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

You'll be surprised how much it grows on you..

Copy link
Member

Choose a reason for hiding this comment

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

Try to wrap .rst files at about 80 columns.

==============

.. dfhack-tool::
:summary: List information related to deites and religions
Copy link
Member

Choose a reason for hiding this comment

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

summary has to end in a period for the in-game help parser to pick it up correctly.


.. dfhack-tool::
:summary: List information related to deites and religions
:tags: fort inspection
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 units would belong in the tags too, wouldn't it?

==============

.. dfhack-tool::
:summary: List information related to deites and religions
Copy link
Member

Choose a reason for hiding this comment

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

I'd call this more than just a "List". How about "Interactive religion, deity, and worshiper browser."?

Comment on lines +28 to +29
``-help``
print in-game help message
Copy link
Member

Choose a reason for hiding this comment

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

you can support a help param (and many scripts do), we just don't generally document it since the canonical way to get help is help commandname

gui/cult-lists [<options>]


Options
Copy link
Member

Choose a reason for hiding this comment

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

In order to present the APIs consistently between scripts that use argparse.processArgs and argparse.processArgsGetopt(), multi-character ("long") options should be documented with two leading dashes. argparse.processArgs still supports the single-dash format for compatibility, but it shouldn't be in the docs.

@@ -0,0 +1,58 @@
cult-lists.lua
Copy link
Member

Choose a reason for hiding this comment

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

It kind of feels like this script could be the home for all religion-related functionality. how about calling it gui/religion?

Comment on lines +31 to +49
``-nogui``
disables the GUI

``-console``
prints results to console plus a number of options
default state prints all deities and their followers

``-printreligions``
prints all religions and their members

``-printunits``
prints all units and their personal deities

``-printall``
prints cults list, worship list and unit list
warning: large forts will exceed the console history limit.

``-showids``
prints religion's history entity ids, deity's history figure ids and unit's in-game ids
Copy link
Member

Choose a reason for hiding this comment

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

just so I understand, what is the intended use case for the text dumping? who is the consumer that is not using the GUI to browse the info?

Copy link
Member

Choose a reason for hiding this comment

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

I haven't looked through this code yet, but could you address the lint errors. Also, please use four-space indents, no tabs.

@myk002
Copy link
Member

myk002 commented May 3, 2023

@gero-pardo have you had a chance to review the comments? I'd love to get this into the next beta release.

@gero-pardo
Copy link
Author

gero-pardo commented May 12, 2023

My apologies! I'm unable so far. Got mid-terms and a special final just shoved in for the end of this month. I'm in no way abandoning the project, but unfortunately this is how my general schedule works - summer's free, winter's not so much.

I'll chime in as soon as I can spare a week, but even then I won't be able to pull in as many hours as I did at the start.

@lethosor lethosor mentioned this pull request Oct 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
2 participants