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

Don't transfer surface/content flags with Alt+click and related shortcuts? #3228

Closed
ericwa opened this issue May 17, 2020 · 8 comments · Fixed by #3260
Closed

Don't transfer surface/content flags with Alt+click and related shortcuts? #3228

ericwa opened this issue May 17, 2020 · 8 comments · Fixed by #3260
Labels
Prio:3 Low priority: Minor problems and nice to have features Type:Enhancement New features

Comments

@ericwa
Copy link
Collaborator

ericwa commented May 17, 2020

This is something muk and @neogeographica both brought up on Discord - when mapping for Q3, the Alt+click family of mouse shortcuts transfer the surface and content flags, and it can be undesirable to have these be transferred along with texture alignment.

My argument is that we've mostly developed and had feedback on these from Q1 mappers, where there are no face flags, and so the only thing these shortcuts do is transfer texture alignment.
e.g. consider there's some architecture that is a mix of detail and non-detail (say a doorway with arches and trim), in Quake you can Alt+click+drag to transfer texture alignment however you like, and the detail/world state is unaffected since that is distinguished by func_detail entities.
But in Q3 it's stored in a content flag, it would get transferred along with the texture alignment.

As a replacement I'd suggest new "Paste surface / content flags" menu item, for those cases when the mapper does want to transfer flags. E.g. you'd select the source faces, press Ctrl+C. Then select the destination faces, and executing this command would only transfer the flags.

@ericwa ericwa added Type:Enhancement New features Prio:3 Low priority: Minor problems and nice to have features labels May 17, 2020
@neogeographica
Copy link
Contributor

This absolutely sounds to me like the correct change in the case of "content flags" like detail, which I think typically need to be uniform for all faces on a brush anyway to avoid weird behaviors. Although TB does support setting them per face, it seems unlikely that you would want to change "content" when painting 1 of a brush's faces.

For "surface flags" I'm less sure.

My main familiarity is in Quake 3 where all the "surface flags" kind of stuff is actually handled in the shader, so those kinds of attributes WILL get copied when the texture is copied regardless.

@neogeographica
Copy link
Contributor

Thinking about this some more...

Since netradiant-custom is the other editor I have handy, I'll mention it as an example again.

When editing a Quake 2 map and opening the surface editor:

  • "detail" is shown in the contents flags editor, but its value cannot be modified from there. (Hardcoded behavior in SurfaceInspector::BuildDialog.) Detail flag can only be changed by toggling it on a whole brush.
  • When copying a texture between faces, all of the surface and contents flags EXCEPT detail are copied. (ContentsFlagsValue_assignMasked)

Not saying that's the best/only approach, but FYI FWIW. It is admittedly ugly to specialcase that one flag in that way. I'd be curious to see what JACK or other Quake 2 capable editors do.


Another data point: In the qbsp3 tool for Quake 2, the function BrushContents prints a "mixed face contents" warning if any faces have different contents flags. It largely treats the contents flags of the first face as the contents flags for the brush as a whole, modified a little bit by checking all face's texinfo for translucent faces: https://github.com/id-Software/Quake-2-Tools/blob/master/bsp/qbsp3/map.c#L291

In q3map/q3map2 for Quake 3 on the other hand, the behavior appears to be that a brush will be treated as structural if any face is structural.

So the compilers don't crash or anything if you have mismatching contents flags, but you may get unexpected behavior.

Mismatching surface flags doesn't appear to be a problem.


At first glance it seems like copying surface flags along with the texture might be OK, although it would also be OK to provide a paste method that does NOT do that.

Copying content flags on texture paste is probably not OK, and it could even be good to enforce that content flags are changed only on a whole-brush basis. Or at least maybe warn about mixed content flags as a map issue?

@neogeographica
Copy link
Contributor

neogeographica commented May 24, 2020

If there's a pretty straightforward solution desired here like "don't copy any flags" (e.g. neogeographica@ef8d036) or "copy surface flags but not content flags", I can take this issue and generate a PR.

@ericwa
Copy link
Collaborator Author

ericwa commented May 24, 2020

Yeah, it's really hard to know what to do. I'd say the choices are:

  • "don't copy any flags"
  • "copy surface flags but not content flags"
  • have a { "dontCopy" : true } in the Q3 game config for detail to emulate Radiant.

One thing to note for Q2 is once we fix #3013 there should be less need to touch flags, because it'll allow the flags from the textures to be seen by qbsp. AFAIK we currently always write the flags/contents/value into the .map so we're clobbering whatever is in the textures.

I did check out what Quark and JACK do:

Quark: The context menu "Texture->Wrapping->From Tagged" only transfers the texture and alignment from the tagged face, no surface flags/content flags/value. (not sure if there is some way to copy them that I missed, other than manually editing the checkboxes)

Jack: you can choose different modes (at least when you click on a face in "Lift + Select" mode, then click on another face in one of the "Apply" modes) so it either copies contents/flags/value, or doesn't, depending on the mode.
jack

@kduske
Copy link
Collaborator

kduske commented May 24, 2020

I would say we should copy surface flags / values, but not content flags, unless the user transfers textures via the double click method which applies them to the entire brush. This should transfer the content flags.

We could debate removing content flags from the face attributes altogether and make them only editable per brush somehow. Or always apply content flags to all faces of a brush automatically.

@neogeographica
Copy link
Contributor

neogeographica commented May 24, 2020

First a quick observation about @ericwa 's "three choices" above:

My current thought is that "don't copy ANY flags (including surface flags)" is likely to seem confusing/inconsistent since surface-flags-like stuff can be transferred along with the texture anyway -- Q3 shader attributes, Q2 defaults from the .wal.

So that narrows it down to the latter two choices IMO.

(Edit: altho, do Q2 defaults also include content flags? hmm, and argh)


Thoughts about inferring intent from double-click: I'm skeptical about how often intent to transfer texture is tied to intent to transfer content flags. In the "detail" case I could definitely see this causing surprising results. E.g. if I pick a texture from some handy visible surface in order to retexture some other brush's faces with double-click, and it ends up changing the detail-ness of the target brush (which I may only discover later/accidentally).

But this is just from my Q3 experience using detail brushes. For the non-detail content flags used in other games like Quake 2 I don't have as good of a sense about what's expected. I guess it doesn't help that the three other editors mentioned here do it in three different ways. :-)


Could punt on the decision with a preference or even a toolbar button that controls the style of flags-copying.

@kduske
Copy link
Collaborator

kduske commented May 24, 2020

I vote no preference. We should not copy the content flags at all, neither on single nor on double click. We could introduce smart tags that allow setting content flags using keyboard shortcuts as a separate feature though.

@ericwa
Copy link
Collaborator Author

ericwa commented May 24, 2020

Yeah I'm also happy with the "copy surface flags only, not content flags" option.
That handles the Q3 case as desired (never transferring detail state).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Prio:3 Low priority: Minor problems and nice to have features Type:Enhancement New features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants