Enhance the multitagging functionality added by !1485

mentioned in merge request !1485 (merged)

added 1 commit

5090de58 - Remove leftover debug printfs

added 1 commit

2d4411d0 - Remove empty 'else' block (also leftover)

A small nitpick: I would suggest using the Netgame Only and No Netgame flags instead*. Flag usage across the many linedef actions is a total mess, but using those specific flags would be consistent with other actions that are typically used in-level, such as slopes and flat alignment.

* Use Effect 6 if a third flag is needed.

added 1 commit

f8fe763d - Fix linedef type 99

Compare with previous version

I thought those flags would automatically have their netgame-related functions, but I guess not. Still though, what if we later decide it would be useful to have the netgame/non-netgame toggles work on these actions?

Fair enough, the current flags are fine.

However, speaking of flags: I was thrown off by the Zone Builder configuration's flag labels, as they are the same across all four linedef actions, despite action 96 using the offsets for target tags instead.

To me, using the texture offsets as additional tags seems much more useful, and I don't see why action 96 shouldn't have access to that functionality. Perhaps another flag could be used as a toggle, with additional tags as the default for consistency with actions 97-99?

You know, I don't think there's really any need to have it be toggled by a flag for 96, since in that case you can always just set the offsets to 0 if you don't want extra tags, right?

Another thing this means is that we have a lot more flags available to use without conflicting with other purposes. So this might be overkill, but what do you think of redoing it like this?

Linedef Special 96: Distribute Tags

Flags:

Tag is source
Apply front tag
Target front tag
Target front sector
Apply back tag
Target back tag
Target back sector
Front X is source
Front Y is source
Back X is source
Back Y is source

The "* is source" flags mean that if it's checked, it'll apply that tag; otherwise, it will target it. Also, the front/back sectors would be treated separately from other sectors with the same tag, meaning, for instance, if you have "Target front tag" checked, but not "Target front sector", it will apply the source tags to all sectors with the same tag as the front sector, except for the front sector itself. This could be useful if you're using the same control sector for something else as well.

And here are all the flags available, even leaving the netgame flags reserved:

Block Enemies
Upper Unpegged
Lower Unpegged
Slope Skew
Not Climbable
No Midtexture Skew
Peg Midtexture
Solid Midtexture
Repeat Midtexture
Effect 6
Bouncy Wall
Transfer Line

So there's still one to spare.

What do you think of this idea? If you like it, how do you think the flags should be assigned? Again, it might be overkill in terms of functionality, but as long as it won't result in code bloat, it seems like something we might as well do to cover as many potential use cases as possible.

I like this merge request, it's a logical extension of my original feature. Also, it's quite easy to use all the texture offsets with these linedefs without worrying about messing up your wall offsets, like so (notice linedef 97 in the middle):

In fact, I originally intended for the texture offsets (and possibly even the texture fields) to have an extra function in this formation, and that's one of the reasons why I chose a linedef instead of a thing.

Anyhow, I personally wouldn't implement using all the flags on a single linedef because that many flags seems messier and harder to use. I can't think of a usage for having a linedef be both a source and a destination, for instance. Analogously, sector-based slopes for combinations of floors, ceilings, backs, and fronts all use separate linedef special numbers. There could be merit in a separate, advanced linedef using the proposed flag idea (say perhaps a linedef type 95 if other people support the idea), but I think the existing ones should be kept in any case for ease of use.

I agree with v-rob on this, using that many flags seems messy. If you do want to implement an extra action for additional use cases, you could cut down on the amount of flags used in your proposal, since using a tag as both a source and a destination indeed seems unnecessary.

Anyway, such an action wouldn't address my main issue, which is that the flags have different behavior for action 96, despite the labels in Zone Builder being identical to action 97-99's. My initial suggestion was to make the regular, no-flags-checked behavior consistent across all four actions, but if you really don't want to do that, you should at least change the labels in the ZB configuration to reflect the difference in behavior.

Personally, I would still prefer action 96's flags having the same effect as 97-99's. When testing all these new setups, I thought action 96 was broken at first, as I'd gotten used to using the offsets as additional tags with the other actions, and spent half an hour debugging it to prepare a bug report for this merge request...

Yeah I understand; I actually wasn't sure about it either for the same reason.

I'll set it up to work the same as 97-99, and then have another flag to have it work as target tags instead. How does that sound? If that's good, then what flag should I use?

That sounds good! In that case, if you want to keep the netgame-related flags free, I guess the next flag to use would be Repeat Midtexture (Effect 5), since the action doesn't need to be in-level. Alternatively, Bouncy Wall could also work, but then the toggle would be weirdly sandwiched between the other two flags...

added 1 commit

0b51b391 - Make 96's flags consistent with 97-99 by default

Compare with previous version

Alright, there you go. :) Here's another test map: apply3.wad

I also changed the descriptive text for the flags to only have the first word capitalized, as that's how they normally are written.

Hmm, I'm getting a warning when compiling...

p_setup.c: In function 'P_LoadLevel':
p_setup.c:2994:16: warning: array subscript 4 is above array bounds of 'mtag_t[4]' {aka 'short int[4]'} [-Warray-bounds]
 2994 |     offset_tags[4] = (INT32)sides[lines[i].sidenum[1]].rowoffset / FRACUNIT;
      |     ~~~~~~~~~~~^~~
p_setup.c:2986:11: note: while referencing 'offset_tags'
 2986 |    mtag_t offset_tags[4];
      |           ^~~~~~~~~~~

added 1 commit

172454f1 - Fix offset_tags array indices

Compare with previous version

Oh, whoops! Just committed the fix; try now.

That warning was fixed, but now there's another:

p_setup.c: In function 'P_AddBinaryMapTags':
p_setup.c:3001:48: warning: suggest parentheses around '&&' within '||' [-Wparentheses]
 3001 |       if (matches_target_tag || offset_tags[k] && Tag_Find(&sectors[j].tags, offset_tags[k])) {
      |                                 ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Okay, I'll fix it when I get home; sorry. I hadn't been seeing any warnings, strangely. Are you using GCC?

added 1 commit

115254ef - Fix compiler warning related to precedence

Compare with previous version

See if that works. I can't test myself right now as I'm having compiler issues.

That works!

merged

mentioned in commit d040b61e

changed milestone to %2.2.10

That's what I get for coding late at night when I should really have been sleeping

added Enhancement label

Enhance the multitagging functionality added by !1485

Merge request reports

Activity