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

Select sectors by distance. #519

Closed
wants to merge 1 commit into from

Conversation

morphles
Copy link

@morphles morphles commented May 7, 2016

When selecting sectors by poin, select sector containing point and closest to
given point, instead of just selecting first one containing it. Allows working
with sectors inside sectors, needed when working with 3d floors or similar.
Use custom distance code, as MapSector::distanceTo seems to be very slow
(noticeable lag on hover highlights if using it).

SIDE NOTE:
Considering some performance implication of this, and the need of this to work 3d stuff (not entirely sure it is strictly needed, as I'm new to mapping, and maybe there are some other methods, but in any case such precise selection seems like a correct way to function), highlight feature might be problematic (though with this distance code it was not bad on my smallish map, while distanceTo was very slow).

When selecting sectors by poin, select sector containing point and closest to
given point, instead of just selecting first one containing it. Allows working
with sectors inside sectors, needed when working with 3d floors or similar.
Use custom distance code, as MapSector::distanceTo seems to be very slow
(noticable lag on hover highlights if using it).
@morphles
Copy link
Author

morphles commented May 8, 2016

After working more I see that highlight is very necessary. And for not too complex maps I tried to create so far there were no issues with this implementation. Though I noticed that working with 3d sectors is still quite complicated in slade, as 3d editing does not seem to support rendering it correctly. Still this is a minor improvement, allowing one to work in some cases. Though there are limitations with this too, if one say has stacked two, say rectangle sectors, on one another, all vertex distances will be same and which one gets selected will be a bit unpredictable (well should be one with lower id). Though I'll try to come up with some better idea for such cases too, maybe.

@eevee
Copy link
Collaborator

eevee commented May 8, 2016

I'm not sure if changing the semantics of a function called sectorAt is a good idea — this would change the interpretation of, say, a sector effect thing in the void.

@eevee
Copy link
Collaborator

eevee commented May 8, 2016

By "sectors within sectors", do you mean self-referencing sectors? You don't need those to make 3D floors, unless you mean purely vanilla invisible floors rather than the ZDoom/Eternity feature.

@morphles
Copy link
Author

morphles commented May 8, 2016

No, I have not worked with self referencing sectors at all. I mean stuff like in his video on doom builder (which i can not use as I use linux) https://www.youtube.com/watch?v=5rsLrwFlhME . You have sector, then you draw in some shape inside, then some dummy sector in some other place, take one line of dummy sector set it to type 160 etc... And you get platform or maybe walls from drawn blueprint, and then roof, then you could add second floor etc etc. Still without support in 3d edit it is quite a pain. And as mentioned previously overlapping vertices is problem

@eevee
Copy link
Collaborator

eevee commented May 8, 2016

I guess I don't understand how this change helps with that? Selecting inside the inner sector should always only select... the inner sector.

@sirjuddington
Copy link
Owner

I'm not seeing the point of this either - MapSector::isWithin will work in all normal situations, the only place I can see where it might not is self-referencing sectors, which you have said this isn't for.

@morphles
Copy link
Author

morphles commented May 9, 2016

Have you tried how current code works? Simple test case, draw a sector, for simplicity a square, then draw another sector inside it. De-select everything, go to sector mode and now try to select smaller sector using mouse. There is no way to do that. As current code bails out as soon as it encounters first sector that contains mouse and returns that sector. As bigger/containing sector was drawn first, it has lower id, and will always be returned. This creates a really awkward situation with basically unselectable sector. I can imagine that being an issue even without trying to do what I was doing. But in any case, for use case see my linked video. If you accidentally place smaller sector inside larger, you will again have trouble removing it (barring undo) or maybe editing some map that has such sectors. I do not see how having such a limitation is good.

@sirjuddington
Copy link
Owner

Are you drawing the middle sector counter-clockwise (so that the lines point outwards)? If so, that will put a 'hole' in the surrounding sector, rather than create a new one in the middle. If you press Insert in sectors mode with the mouse inside the smaller sector it should create it.

@morphles
Copy link
Author

morphles commented May 9, 2016

I drawn sectors in both ways. In any case when I draw sector inside walls seem to be two sided by default. As for the hole, if I'm using 3d floors (which I am) it is not a hole but floating platform or some such. Again look at the video. I can upload wad with my tests after about 10hours when I get back from work. In any case look at the orginal code of SLADEMap::sectorAt not sure how line order matters there. Also As for drawing inside, I can draw sector outside and then move it inside some other (though that seems to have some "interesting results" in game). After such move I can not move it out (without undo) as I can not select it anymore. Again old code just check till it finds firs sector containing given point, that is used to highlight sectors on mouse over in sector edit mode. Then clicking selects highlighted sector, but as you can not highlight sector with larger id that is completely inside sector with lower id (should be obvious from old code why, if (sectors[a]->isWithin(point)) **return** a;). This in my opinion is serious limitation if not a bug. Again if you haven't watched (and from you'r comment I'm guessing you did not, as after watching it, it should be obvious what I want to do - free floating platforms or multi-floor buildings and similar) please watch a video, or maybe after I give you my wad it will be more clear.

@sirjuddington
Copy link
Owner

I have watched the video, and what he does with the inner sector works fine in SLADE, the inner sector is selectable.

isWithin doesn't only check the bounding box, it looks at the lines as well. In the case of a point within an inner sector, the isWithin function on the outer sector will fail, as the point is on the wrong side of the nearest line to be within the outer sector.

I think I'd have to see your map to figure out what's going on on your end.

@morphles
Copy link
Author

morphles commented May 9, 2016

I'll test again with old code when I get back. But I believe it did not work for me. But what definitely does not seem to work is creating smaller sector outside and then moving it inside bigger one, then trying to select it does not work for sure.

@sirjuddington
Copy link
Owner

Yes that is expected, doing that will result in the inner sector's lines having the wrong sector references, as SLADE doesn't automatically fix/update these in this case.

@eevee
Copy link
Collaborator

eevee commented May 9, 2016

It probably should. :)

@morphles
Copy link
Author

morphles commented May 9, 2016

Hm I guess this can be closed. Indeed it seems everything seems to be working when drawing new sector directly inside larger sector (no matter the direction, CW or CCW). It only fails when moving in sector from outside, possibly I mostly tried that before patching. But for that as I understand it is unrelated feature. Wonder how I got so lost. Possibly I mostly tried drawing outside and moving in. Sorry for time wasted.

Still looking at that doom maker video and as far as I have tried with slade, it seems slade has some limitations, i.e. 3d view does not seem to show 3d floors as they are rendered in-game or in doom maker. Unless maybe I'm again doing something incorrectly or missing some setting?

@morphles morphles closed this May 9, 2016
@eevee
Copy link
Collaborator

eevee commented May 9, 2016

3D mode doesn't show 3D floors yet, no. I've done some work on it; it's #205.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants