[CI] Flexible zone validation and registry builds

[zerosenses]

#10

[Slackaduts]

I'm pretty sure this would only work for top-level zone distinction, like worlds. We actually want any level of distinction down the chain, so something like this would likely work best:

def load_zones():
    zones = set(json.loads(ZONES_FILE.read_text(encoding="utf-8-sig")))
    prefixes = set()
    for z in zones:
        parts = z.split("/")
        for i in range(1, len(parts)):
            prefixes.add("/".join(parts[:i]))
    return zones | prefixes

[zerosenses]

I strongly believe it is only needed for top-level zone distinctions as each zone is already added in zones.json.

However, tier zone distinctions and special event zones such as PoP (Portal of Peril) gauntlet (sigil) zones have different zone names than the housing gauntlet they're being hosted as: I don't think you should stress this but its mentioned while on topic. (In short, some zones noticed missing. But the most irrelevant ones is my strong guess.)

Back to the main point. Take this as an example for someone who wrote a bot and wanted it included in all the interior zones Aquila/Interiors: It would probably be a silly user choice to pick that as their @zone since they could just make it Aquila wide. Interiors are found in different dungeons/sigils and they do not follow the same logic as a top-level world zone. Of course, ill be honest I didn't extensively look into your reply, but it just adds much more room for fu***d up bots if it allowed for intermediate-level zones to be considered when they're non-existent. We don't want bots badly written in the first place, let alone someone who's adding something they do not know where it's gonna land on the tool's Bot Search Compatibilities.

To assist my take, it was also surfaced that:

if errors:
        raise SystemExit(f"refusing to build registry: {errors} invalid bot(s)")

Could be replaced with:

if errors:
        print(f"\nWARNING: {errors} invalid bot(s) excluded from the registry "
              "(see FAIL lines above). Fix them to get them listed.", file=sys.stderr)

What this replacement does to scripts/registry.py is such when a bot gets merged/accepted, the following CI "Validation before publishing" comes out False, bottlenecking the entire sequence of merged/accepted PRs after that, until it gets addressed first/fixed.
Why not make this change?
Well, imagine accepting/merging a PR/bot from someone that seemed completely fine, passes the checks, fails on publishing to index.json again (meaning the bot is not searchable on the tool, yet exists as a file in the designated directory on github's bots/bots/..). Now, a month later the repo receives a few more successful commits that pass CI and become part of the index.json bots (searchable on tool). Don't you think it'll be forgotten and let alone? That one slop of a bot that got accepted/merged that does not show-up on deimos's "Search Compatible Bots" even when the Client and Zone requirements were met from the deimos user's end? It will live in the directory bots/bots/.. and will be forgotten.
So my affirming point being made, is so that a firm bottleneck can be helpful, instead of fully accounting for more things. Less is more type thing.

Any way this is just my opinion, and again, im not a zones expert. But, I will say, each bot has a sensible starting point/zone to be registered in. Unless the bot holds enough reason, built for that compound goal.

I would say, a different iteration would be to allow for multiple zone name additions to the @zone = metadata. But then, this is a more hectic take where other things are likely to be changed for its final stage. First, you would not want multiple duplicates of a single bot to be shown in bots/bots/.. folders just bc someone decided to add multiple zone names to their PRed bot. The idea from multiple zone names is for the tool user to search the same bot in those multiple zone names, without disdaining the directory on github with the same bot file in different subtrees.

So maybe a fix like giving room for the publisher to add a lang such as @zone = WizardCity/WC_Streets, sub_zone = Celestia/CL_Hub. This would consider the bot to be registered to WIzardCity/WC_Streets in the dir bots/bots/WizardCity/WC_Streets and then only show the bot from search when standing in Celestia/CL_Hub or WizardCity/WC_Streets, without adding a duplicate bot of its kind to bots/bots/Celestia/CL_Hub: Retaining both zone compatiblities.

[Slackaduts]

I think a "zones" optional field would be best suited for this and a new checkbox in the publish UI to toggle it would be the optimal solution.

However, unless I am misreading I think you misunderstand my point. Its not actually our distinction to make, we should simply have a very permissive system. Take the case of https://github.com/Deimos-Wizard101/bots/tree/main/bots/WizardCity/WC_Streets, I could just do WizardCity/WC_Streets if I want a bot to apply to all of those zones. I don't know why anyone would actually want to do that but there's no reason why we should deliniate based on world/etc.

Now perhaps we also need a GUI hint for doing world-based deliniation, which is the most common outside of zone-specific obviously. However for the purposes of CI validation and searching strictly I think we should allow as much as possible within the hierarchy of KI zones.

[zerosenses]

I understand that point. I dont necessarily disagree with the broad idea.
The allowance/addition of more values in the publisher's favor, is just gonna require more CIs to account for all the messy probable outcomes. Such as the zones that are in no correlation logically to each other, yet are under the same zone-name at some level. [https://github.com/Deimos-Wizard101/bots/tree/main/bots/Aquila/Interiors]
Despite everything, I assist your train of thought. You left me somewhat in agreement with less to say. All is left now is to code.
And, 3.14.0 works great, like the new stuff that's been pushed recently altogether.

[Slackaduts]

Such as the zones that are in no correlation logically to each other, yet are under the same zone-name at some level.

We leave this to the responsibility of the bot author. And I can assure you not a single bot can be created that works in 100% of zones from a particular world, so this argument isn't self consistent.

We will of course have false positives but this is why the end goal is a "zones" option. Deimos pre-fills the current zone so the only way this can be messed up is if the publisher modifies the string after the fact.

[zerosenses]

It does sound more permissive the way you put it. However, this will look complete from my POV with the sub_zone toggle option/idea overall. Then the publisher will have both sides where both sub_zone specificalities can be published: 1) Publisher's choice of sub_zone strict bot search zone-compatibility, plus the main zone they think the bot should land their PR in on github. 2) The entite main zone's matching level names, incase of a manual zone publisher input, a mistake, or intentional compound targeting of a level group of zones.
Both that are mainly meant for passing CI validation process.

From a bot_registry.py standpoint from the tool itself:

1. There shouldn't be a worry if search also values the non-existent_passed-validation zones as part of the standing_zone similar to what a top-level world would be treated as.
2. "General" zone bots are the highest level to all worlds even, which is deemed read from any standing point in-game.
3. Specific zone bots will only work with a single zone integration which must match a game's literal zone, also found in zones.json.

Point no.2: Leaves less room for accepting a General zone bot that is not best for General searchability on the tool. Despite the initial publisher's choice permissions or thought process.
This point is definitely debunked if the bots registered in index.json are meant for any and all bot kinds that pass validation regardless. It does also cross by the maintainer's validation first, before any CI validation any way.

Point no.1: If somehow a flawed zone group passes a maintainer's validation unknowingly. CI will still block it if it doesn't find a matching landing zone in bots/bots/.. any way, which is great.
Then, you'll have the niche cases. Where again, the un-related zone matching level groups. That are both found in bots/bots/.. and passes through maintainers merging the PR somehow. Only then an issue may arise for an infested bot that shows up (searchable) in places not supposed to.

Point no.3: From a publisher's standpoint, the cleanest and most precise publishing take there is. Adding the refined sub_zone functionality to the tool, additionally allows publishers' access to more welcoming ground of precise zone integrations. The broad idea will fall short when the user only has permission to a single zone choice. Regardless whether or not, the user is intending a group of the same level zone name.

Sorry I had more to elaborate writing this, i just got back and i think its whole atm. Will edit if any thing.

[Slackaduts]

1. Your wording on this makes no sense IMO, but if I'm understanding correctly I agree.
2. Yes, we want this to be explicit since it shows up for everything- otherwise people could just blindly remove the zone from all their bots and we've defeated the purpose of the system.
3. "zone integration" is implied but there is nothing enforcing this, nor is it our job to.

[zerosenses]

Code will speak better for me. You had understood my exact intention with this PR initially. I'm going to code at my own pace, so bare with me. I'll try my best, hopefully.

[zerosenses]

In correspondence with tool's publisher PR: Deimos-Wizard101/pull/109.