Add support for QuadPoint trigger zones - #243
Conversation
327e7ae to
1781be6
Compare
Retain existing trigger zone compatibility, assume circular if `type` parameter not given. Updated loadtest.miz to include new trigger zone types and fields.
1781be6 to
5190f0b
Compare
|
FYI even quad zones have a radius defined in the miz. Don't ask me what the hell it means, but that property should maybe be re-exposed on the base type to more accurately represent the miz. |
|
I disagree with re-exposing the property, I think a polymorphic method should be added to decide whether a given point is in a trigger zone. That should allow for an easy usage of both trigger zones. As for the implementation details on deciding whether a point lies in a quad-point trigger zone, leave that up to me. I already have a solution in mind 👍 |
|
If the property isn't retained we can't reserialize the miz correctly. |
|
Sorry if I broke something. I haven't looked at this for a few weeks, so bear with me and correct anything that I get wrong here... As far as I recall for quad trigger zones, the radius parameter is only used in the mission editor UI -- it has no in-game function. The previous behavior totally broke quad zones -- I wrote a script to help update my friend's mission, and deserializing/reserializing it via PyDCS broke all of his quad zone bridge destruction triggers. Hence this PR. See #207 for an issue where DCS had a similarly meaningless parameter. PyDCS had elected to keep it. Eventually a DCS update came along causing DCS to no longer emit this attribute, which caused PyDCS to fail to deserialize new missions. Since it seemed useless, and quad triggers work just fine without this parameter, I elected to not include the radius parameter (although providing it as part of the base class did cross my mind). I think my intention was that
@DanAlbert Can you elaborate here? I see two possibilities
Unless I'm mistaken, the only downside to the current behavior is that the mission editor UI looks a little different for QuadPoints. But please correct me if I'm wrong! |
No need to apologize, I think none of us really knew how to handle this properly until now. At the end of the day it's good that we're running into this problem. Your second suggestion is what I have in mind, basically a function to test whether a point is inside a trigger zone. |
I just opened up So that means we'll need both a way to test if something is in the zone, as well as re-expose the radius property. Good catch @DanAlbert 👍 |
No worries, you've broken far less than I have :)
In general, the types in pydcs try to mirror the data in the miz. This is sometimes needed for serialization (as it turns out what the case here, though it isn't always). It might not have a gameplay effect, but those properties can be useful to programs that read an ME generated miz (as Liberation does).
I can take a stab at these, but @rp- is the real decision maker :)
Avoid breaking the API when it's easy and safe to do so, but so far every time I've had a good reason to break the API it's been accepted :) There are not that many (direct) users of pydcs, so breaks usually aren't too hard to accommodate.
Generally want to be able to read any miz, but write the newest schema. The APIs are generally shaped to match the newest schema, but I'm sure there are exceptions. I think the answer for "how does pydcs support old versions of DCS?" is "use an old version of pydcs". HEAD usually works for the latest open beta and that's all that's "tested".
Watching this project is what I've done. I don't believe there's a discord for pydcs, though it might be time to create one? Up to @rp- I think.
@Raffson beat me to it: gameplay isn't always the only thing that matters :)
Cool, thanks for verifying that 👍 |
Sorry, I still don't fully understand the use case... can you guys give me an example of where a user would need this test? |
Re-exposing the radius is needed in order to prevent the ME from getting bugged if you open the mission and wish to manipulate the trigger. The "test" I referred to is needed in Liberation. I believe it's scenery_group.py that used to test if "white trigger zones" are located within a "blue trigger zone" by using radius. For quad-points this wasn't necessarily a correct determination, and now that there's a distinction being made it would make sense to have polymorphic methods for both circular and quad-point zones to check if something is located inside the zone. For quad-points this proves to be somewhat trickier. Yes, I know there's another method that tries to compare areas of triangles that can be formed between the points, but I'm afraid things get just as complicated as my current implementation in case of angles that are bigger than 180°, sort of like the second screenshot I uploaded in #251. |
|
Ok, thanks... still trying to understand... is the idea to find out whether the intersection between two polygons is nonempty? Or a polygon and circle? Does this functionality really belong in PyDCS? Can/should liberation implement this routine separately? |
I created a method that tests whether a point is inside a zone (so nothing like checking for intersected zone or such, yet...), so it's pretty general. The point in question could be the location of a unit, the center of another trigger zone like Liberation does, etc.
Imo yes, pydcs is more a library than an application, and good libraries usually provide a bunch of useful things with it, and I would say adding the capability to test if a something is inside a trigger zone falls under "useful things". Though I really wonder, why do you doubt whether this should be included in pydcs? Is there a particular reason for your doubt? |
|
|
||
| # DCS mission format misspells the plural of "vertex". We follow this convention within PyDCS. | ||
| class TriggerZoneQuadPoint(TriggerZone): | ||
| def __init__(self, _id, position: mapping.Point, verticies: List[mapping.Point], |
There was a problem hiding this comment.
What is this position? I know that the miz needs this for serialization, but I don't understand if there are any restrictions on how I pick it.
There was a problem hiding this comment.
Sorry, it's been a couple months since I've looked at this, but I believe this parameter refers to the icon location in the mission editor. e.g. the "X" icon in the image below.
Here I modified one of the unit tests to create a quad trigger zone centered in Sukhumi, but place the position parameter on top of Gudauta. You can see that the name and icon for the zone are over Gudauta, while the actual trigger zone area is centered on Sukhumi.
caucasus = dcs.terrain.Caucasus()
m = dcs.mission.Mission(terrain=caucasus)
zone_center: dcs.mapping.Point = caucasus.sukhumi_babushara().position
other_point: dcs.mapping.Point = caucasus.gudauta().position
ZONE_RADIUS_M = 30000.0
offsets = [(ZONE_RADIUS_M, 0.0), (0.0, ZONE_RADIUS_M),
(-ZONE_RADIUS_M, 0.0), (0.0, -ZONE_RADIUS_M)]
offsets = [dcs.mapping.Vector2(*o) for o in offsets]
verts = [zone_center + o for o in offsets]
m.triggers.add_triggerzone_quad(other_point, verts, False, "quad zone")
m.save('missions/test_quad_trigger_zone.miz')
As far as I know, this position only has an effect on the editor and not to gameplay. If I edit the zone vertices in the mission editor, DCS moves the marker (and thus this position parameter). It seems like DCS places the icon at the centroid of the convex hull of the quad.
There was a problem hiding this comment.
Ah, alright then. I'll push a change to document that behavior, and potentially just remove it from the API? It seems doubtful that anyone making these zones actually cares, so we could just place it arbitrarily at the midpoint and simplify the API.
There was a problem hiding this comment.
That's a good call. I guess I can't really think of a use case (since as soon as the user edits the zone in the ME the marker will be repositioned). But perhaps expose the position parameter optionally, defaulting to the centroid if unspecified?
4 participants
Retain existing trigger zone compatibility, assume circular if
typeparameter not given.Updated loadtest.miz to include new trigger zone types and fields.