Unable to add World to Trigger filter

[LCWilliams]

Problem:

Adding the worlds filter to a trigger fails, and produces the below type error: TypeError: issubclass() arg 1 must be a class

Traceback:

  File "/mnt/500SSD/Projects/planetside-discord-bot/src/planetsidebot.py", line 87, in setupContTracker
    await self.contTrackerCog.CreateTriggers()
  File "/mnt/500SSD/Projects/planetside-discord-bot/src/ps2ContinentTracker.py", line 211, in CreateTriggers
    self.auraxClient.add_trigger(
  File "/home/lee/.local/share/virtualenvs/src-rIqK2qin/lib/python3.10/site-packages/auraxium/event/_client.py", line 117, in add_trigger
    subscription = trigger.generate_subscription()
  File "/home/lee/.local/share/virtualenvs/src-rIqK2qin/lib/python3.10/site-packages/auraxium/event/_trigger.py", line 216, in generate_subscription
    and any((issubclass(e, CharacterEvent))  # type: ignore
  File "/home/lee/.local/share/virtualenvs/src-rIqK2qin/lib/python3.10/site-packages/auraxium/event/_trigger.py", line 216, in <genexpr>
    and any((issubclass(e, CharacterEvent))  # type: ignore
TypeError: issubclass() arg 1 must be a class

Reproduction/Code:

        worldToMonitor = await self.auraxClient.get(type_=World, world_id=13)

        if ContinentTrack.contLockMessageType != PS2ContMessageType.NoMessage:
            self.auraxClient.add_trigger(
                Trigger(
                    name="CONTTRACK_Lock",
                    event="ContinentLock",
                    worlds=[worldToMonitor],
                    action=self.ContinentLockCallback
                )
            ) # END: Add trigger- Continent Lock

Notes

1. Behaves the same regardless of whether using a list of world objects, or list of ints (as per the documentation).
2. Omitting worlds=[] allows the trigger to be created and behave as expected. One just has to then check the world_id manually from the event.

Sidenotes:

1. It would be nice if the documentation for this (triggers) could be improved. I've been left scratching my head quite frequently on some things because of the documentation.
2. You can't get a world by name using the non-deprecated functionality, meaning you have to instead get them all then manually filter to get the desired world if you don't know the ID.

[leonhard-s]

Hi, thank you for the report!

Just had a look at this, the issue is isolated to the trigger automatically enabling logical AND behaviour for character-centric events if a world ID filter is specified. While ContinentLock is not a character-centric event, a subclass check is used to test this, which fails if the event is a string.

For anyone needing a quick workaround, using the auraxium.event.ContinentLock class instead of the string "ContinentLock" avoids the issue.

Regarding side note one

Thank you for pointing that out, the use case of manually creating and updating Trigger instances is indeed barely covered. I will see add a more comprehensive example for this when I find time.

Regarding side note two

This is an API limitation that is difficult to work around cleanly; the collection does not allow such lookups. It was decided to deprecate and eventually abandon the per-object overloads for the .get_by_name() and other getter/query methods as they proved difficult to test and maintain. But this also means that they will no longer be available for these convenience helpers.

I would like to restore this utility (and potentially introduce new ones) in a less invasive way. Perhaps via examples within the repository, or via a utility namespace in the package that provides such helpers methods - open for feedback here.

[LCWilliams]

Very welcome! :)

I did manage to find a non invasive fix for this by replacing issubclass with an isinstance (pull requested), which allowed the code to function as expected, though I'm not sure of any side effects this may have. Other triggers created still behaved as expected with this fix in place.

Re: Documentation

I can possibly look at writing up some draft examples/documentation if you're unable to find the time, after looking at the actual code and having a better understanding of it. The bug/s deterred me a bit beforehand thinking I just didn't know how the library worked!

Helpers

Just as a thought, since I'm not sure how you accomplish this already, but I was wondering if you couldn't accomplish this similarly to how you already do the keyword argument search (like world_id=#, but obviously for the name instead).

I previously used the test data in the repository for some existing IDs which were mostly accurate; so that's an option, though a little hidden and sometimes outdated.

I think the main problem that needs to be addressed is having a consistent but reliable way to get objects without knowing their IDs, which could be done by having a ps2ObjectNames module, with intEnum classes for each main type.
The name of an enum entry would just be its en. locale, as it'd be much easier to maintain, and its value being the ID, so that ultimately a user would simply need to: client.get(ps2Object, objectNames.example_type.objects_name)

One could subclass the enum classes so they all have a Populate function which sets the values.
Making this more user friendly could be accomplished in a few ways; if the enum is used and a value is None (default), call the getter- though this could become fairly messy.
Alternatively have a populate function that takes the enum objects as a parameter and calls their respective getters: the user would then simply need to add this function and any of the utility classes they use which would only grab the data needed.

[leonhard-s]

Keeping this open to continue the discussion on side note 2:

• I was wondering if you couldn't accomplish this similarly to how you already do the keyword argument search (like world_id=#, but obviously for the name instead).

  The **kwargs for auraxium.census.Query, client.get(), and client.find() should all accept arbitrary field types, including double-underscores to mark dots:

  item = await client.get(ps2.Item, name__en='NS-11A')

• The name of an enum entry would just be its en. locale, as it'd be much easier to maintain, and its value being the ID, so that ultimately a user would simply need to: client.get(ps2Object, objectNames.example_type.objects_name)

  The main issue with having any kind of in-source index for entity names is that it dramatically increases the maintenance effort for the project. Nearly all in-game events or gameplay updates introduce new items or cosmetics, and all of these come with names and their own IDs. Bumping the version number every time to introduce or update a string constant is not a viable option.

  Disconnecting these IDs from the rest of the project to be run/updated separately would be an option. I already have a project that checks the API for updates every six hours and fetches/updates all entries in JSON format; reading and converting that into the type of enum pattern you suggest would be doable: https://github.com/leonhard-s/ps2-api-backup

---

I believe the easiest solution will be to create another overload of client.get_by_name(World, 'Cobalt') to perform the same workaround the World.get_by_name(...) helper used; fetch all worlds and compare the names locally.

The helpers themselves were not the problem, but having all of the client methods replicated in the objects created way too much coupling between the object model and the client, which is why they are scheduled for removal.

[leonhard-s]

The auraxium.Client.get_by_name(World, ...) overload has been added with v0.3 - thank you for the contribution!