Add selectableKilled modrule to control whether dead units should be selectable

[saurtron]

Work Done

• Make Unit.cpp call selectedUnitsHandler::RemoveUnit on unit::ForcedUnitKill.
• Make Unit also set noSelect when killed.
• Controllable through modrules selectableKilled
  • defaults to true for backwards compatibility

Remarks

• Current behavour is ambivalent, since units get removed from groups but not from selection.
• Seems just an overlook
• This is a real issue because api users get confused by units still in selection even after UnitDestroyed called

How to test

• Get BAR branch from https://github.com/saurtron/Beyond-All-Reason/tree/test-selection-destroy
  • Could make a better test, but this is the one I used to find the issue
• Start skirmish in neverend game end mode
• Start game
• Run /runtests attackrange
  • You might need to run this several times, but finally the assertion will trigger
  • gui_attackrange_gl4.lua has an issue where GetSelection still returns dead units, thus corrupting its logic
  • Might be other affected lua code in the wild not sure

[sprunk]

Engine should leave decisions to Lua, especially when it's trivial to do so there. What if I want to keep a unit with long death animation selected? Just deselect the unit in Lua UnitDestroyed.

[saurtron]

Engine should leave decisions to Lua, especially when it's trivial to do so there. What if I want to keep a unit with long death animation selected? Just deselect the unit in Lua UnitDestroyed.

This is causing real problems and very difficult to debug too.

[saurtron]

Just deselect the unit in Lua UnitDestroyed.

For your use case they can reselect in SelectionChanged. Imo it's much more niche and who knows how many widgets or gadgets have the issue this fixes.

[saurtron]

@sprung see the above issue from bar to see how easy it can be to fix bugs caused by this behaviour.

[sprunk]

1. use this https://github.com/beyond-all-reason/Beyond-All-Reason/tree/sprunk-patch-2
2. deselecting units doesn't solve any bug because you can just reselect the unit, at best it makes them harder to reproduce. Fix your gameside gadget. I would expect the issue to boil down to using UnitDestroyed where RenderUnitDestroyed is supposed to be used.

[saurtron]

2. deselecting units doesn't solve any bug because you can just reselect the unit, at best it makes them harder to reproduce. Fix your gameside gadget. I would expect the issue to boil down to using UnitDestroyed where RenderUnitDestroyed is supposed to be used.

Man, when a unit is killed and UnitDestroyed is called it should be removed automatically from selection, that's the most reasonable behaviour. Why would you reselect it? If you do of course you're on your own with all the problems you would cause in other places. I think that's totally reasonable.

Also I already noted this can result in difficult to track bugs it already cost me hours just to find what was going on, and also possibly other devs as well. Totally the last thing I expected was for the unit to still be there at the selection after I got it removed from visibleUnits and got UnitDestroyed too.

It's removed from basically everywhere.

[WatchTheFort]

Having a destroyed unit selected does not make semantic sense.

[sprunk]

Having a destroyed unit selected does not make semantic sense.

It does at an engine level. But a game could disagree and it could make no sense within its design, in which case it is free to make such units unselectable via 1 line of Lua.

should be removed automatically from selection, that's the most reasonable behaviour.

The most reasonable behaviour is for the engine to let games decide on their own UI design via Lua, and facilitating that design's implementation in the form of "do X" rather than "work around the engine implementing some specific behaviour X".

If you do of course you're on your own with all the problems you would cause in other places.

There aren't actually any problems inherent to selecting dead units. BAR just has some broken wupget that handles it incorrectly, fix it (or use the 1-liner to work around it).

It's removed from basically everywhere.

Not from the places relevant here. Check RenderUnitDestroyed, see when it runs, think on why it exists and how it might help with the rendering issues you're having.

[saurtron]

It does at an engine level. But a game could disagree and it could make no sense within its design, in which case it is free to make such units unselectable via 1 line of Lua.

In the spirit of the "reasonable defaults" I think we should be able to agree the reasonable behaviour is to have them unselected, and then having games wanting otherwise to override that in some way. Otherwise we're just forcing games to workaround broken behaviour.

Actually I'm pretty sure the games must already be working around that since otherwise there would be more problems with all those selected but dead units.

Not from the places relevant here. Check RenderUnitDestroyed, see when it runs, think on why it exists and how it might help with the rendering issues you're having.

Could be to workaround some other broken behaviour, but can't be sure tbh.

[sprunk]

Actually I'm pretty sure the games must already be working around that since otherwise there would be more problems with all those selected but dead units.

A good topic for you to investigate.

RenderUnitDestroyed could be to workaround some other broken behaviour, but can't be sure tbh.

Another good topic to investigate.

[sprunk]

Suggested change

	if (isSelected)
	selectedUnitsHandler.RemoveUnit(this);
	/* This is a hack and should be done gadget-side,
	* because else it's a hassle to keep the unit selected
	* if you'd rather that was the behaviour. */
	if (isSelected)
	selectedUnitsHandler.RemoveUnit(this);

[sprunk]

I had a talk with @saurtron and he doesn't seem to buy my arguments, so for the sake of good relations I won't veto this (i.e. I'll give an "approved" review) and I'll leave the judgement to whoever will be handling it.

That said:

"Bugfix"

• things can happen to units that are dead, i.e. are undergoing their death animation. For example, they still get rendered so shaders need to keep their data past wupget:UnitDestroyed. For this purpose, wupget:RenderUnitDestroyed exists. It is somewhat misnamed and isn't inherently related to rendering. It just runs when a unit is truly, finally gone.
• in particular, selection is another thing that can happen to dead units. Whether that makes sense is a different question that I answer in the next section. As-is though, wupgets dealing with selection should use wupget:RenderUnitDestroyed rather than wupget:UnitDestroyed.
• BAR has some bugs that are apparently caused by selecting units undergoing their death animation. There are claims that this is a problem inherent to the possibility of dead units being selected, but no other game seems to suffer from the same problem and I have seen zero evidence that this is anything but a broken BAR wupget that fails to use wupget:RenderUnitDestroyed properly.
• the PR, as presented, does not actually prevent dead units from being selected (it deselects when health reaches 0, but you can just reselect again) so I don't think it even fixes the problem.

I think the ideal solution would be for BAR to find and fix the broken wupget.

Semantic sense

• there was a claim that selecting dead units does not make semantic sense. At engine level, this is not true since there are plenty of reasons one might want to extend the nominal death animation and keep dead units selectable:
• you could have Age of Mythology style heroes, i.e. units that sort of linger there on death. Selectability them is a natural way of finding them, of figuring out what's that glowing corpse on the ground, of maintaining control groups.
• you could have an ability for kamikazes where they safely fizzle if killed, but you have a small time window where you can order a detonation anyway.
• you could have a button on a fresh corpse to call a live unit to do something (e.g. have a medic pick up a "wounded" soldier like in Foxhole, or choose a specific corpse to raise as undead).
• you could have the selection GUI panel for important units show some useful lifetime info such as what killed me or time until respawn.
• you could have any sort of editor mode (perhaps a full-fledged actual mission editor like SpringBoard, or just a dev tool within a "standard" game) that lets you move any unit, no reason to explicitly block units undergoing death animation.

I don't think there is any reason to enforce this at an engine level.

Gamedev QoL

• there is a claim that the change would improve defaults.
• I wouldn't mind it being the default behaviour if there was an opt-out. As-is, this is a breaking change that does not allow opt-out, other than some complex system that duplicates selection tracking to be able to tell which unit got the deselection that I never signed up for.
• I would expect at least a PoC for a Lua replacement where it is nontrivial. As-is, none is offered.
• meanwhile the current opt-in mechanism is to just place Spring.DeselectUnit(unitID) in a wupget with exactly equivalent behaviour to this PR; or Spring.SetUnitNoSelect(unitID, true) to do it properly. I couldn't dream of a way to do this that is easier or more clear at stating its intent.

I think the engine not doing anything is the perfect design here, and if a game doesn't want dead unit selection (whether to work around its own bugs, or because it doesn't make semantic sense within its game design) it should just place the 1-liner gameside.

[saurtron]

• I wouldn't mind it being the default behaviour if there was an opt-out

hey it's no problem to have an opt-out, we can make this depend on a modrule (or modinfo not sure what you like to call those, or even a config option) and then make that also control removing them from groups (I guess if you want them in selection and selectable you still want them in groups too) and possibly deny being able select them again (just need to check unit->isDead). I'm just not sure not preventing/removing them from selection is a real need, and when it is that can be added easily... so safe to leave it in a certain way atm till someone requests it, but if it is for you then for sure the modrule thing is the way to go.

Keep in mind by the time someone requests this the engine could already be improved, be more modular and not even be an issue any more.

Being able to select them is certainly a problem and I'll look into that too and see what can be done generally... I fear the root of the problem is they shouldn't even be units any more or at least not be in the general units arrays, but don't know the specifics atm. Anyways just need to unit->isDead in a few places.

I think the dead-but-alive thing looks like a workaround that must cause a whole lot of issues.

[GoogleFrog]

I agree with @sprunk, this PR seems to exist to deal with bad gameside code. The PR adds arbitrary behaviour that would be very hard for a game to work around adequately, while the "fix" desired by the PR is simple to do with well-written lua. Is the difficulty with how BAR handles preselection? I had a look at the BAR preselection rendering a while ago, and I think it has fundamental issues. If that is the case, then fix the BAR code rather than make the engine worse via adding hardcoded behaviours.

Things "shouldn't just be a modrule", because then we'll end up bloating modrules with tiny tweaks like this, absolutely confusing anyone trying to develop a new game, and confusing cross-game communication. The engine can, and should, be general without a bunch of modrules for things that the lua API already handles quite well. Modrules should be reserved for cases where the engine has to make a choice between doing two expensive in-engine operations, not to save gamedevs the work of doing a 1-line change in a gadget.

[saurtron]

hey! thx for chiming in.

The PR adds arbitrary behaviour that would be very hard for a game to work around adequately, while the "fix" desired by the PR is simple to do with well-written lua. Is the difficulty with how BAR handles preselection?

I'm not entirely sure about that, because like @sprunk commented and I intend to also work on here, it's not just about removing the unit, but also about preventing it from being selected again.

Also, it's not just about the selection, since I believe the same behavior should be implemented for groups (ie, both removed when unit killed, or none). Right now when dying the unit is removed from groups, but not from the selection. If there aren't even more problems now is probably thx to the engine actually removing units from selection groups when killed.

Also probably there are other places where the engine could provide a more consistent behaviour for dead units, like not return them in Spring.GetAllUnits() for example.

There's another problem, and it's that if there was some c++ EventClient listener running before lua, then I think it would be impossible to do a perfect "fix" from lua. I don't think there's one now, but it's not farfetched to think there could be one in the future, so leaving this unsolved could be a problem down the road.

All in all, I think providing an option to enable killedUnitsBehaveAsDestroyed, or however it could be called, is the way to go. This option should be about a comprehensive behaviour for making the engine do the expected thing in all situations, not just the selection issue pointed at here. I still don't see the need to have those dead units in selection tbh, and I think this should just not be an option, but since we disagree in that, then having an option to override is the way to go tbh.

Things "shouldn't just be a modrule", because then we'll end up bloating modrules with tiny tweaks like this, absolutely confusing anyone trying to develop a new game, and confusing cross-game communication.

I agree not everything should be a modrule, but for this, I do believe something will be needed.

What I would like to hear, is whether modrule or config option would be preferred in this case. Probably a modrule since this is probably affecting synced too.

If that is the case, then fix the BAR code rather than make the engine worse via adding hardcoded behaviours.

I think the fact BAR has a lot of problems with this points to the fundamental issue of how much burden this inappropiate default behaviour, and the fact there's no easy way to override, is putting on games.

the work of doing a 1-line change in a gadget.

Its not a 1 line change in a gadget, this bad behaviour needs tweaks everywhere. It should be an independent gadget running before all others, that's for starters, and also it's not just about removing from selection but avoiding it to be reselected again, as well as avoid the dead units from appearing in unit queries in a transparent way, ie, not having to go look for all GetAllUnits calls to put GetAllUnits(false) or having to hook that at synced, unsynced and ui to make it behave properly.

See for example:

• https://github.com/beyond-all-reason/Beyond-All-Reason/blob/502883f4ea4f0d365517721eab2a863734ffe825/luaui/Widgets/gui_defenserange.lua#L616.
• https://github.com/beyond-all-reason/Beyond-All-Reason/blob/502883f4ea4f0d365517721eab2a863734ffe825/luaui/Widgets/gfx_airjets_gl4.lua#L757
• I see problems with allyground_attackrange_gl4, allyAA_attackrange_gl4, selectedUnitsVBO, allycannon_attackrange_gl4, DrawUnitGL4, jetInstanceVBO, allynano_attackrange_gl4, losrangeVBO, enemycannon_attackrange_gl4. I'm not completely sure since I couldn't debug all those yet, but atm but I'm starting to think those are because of GetAllUnits returning dead units. It's a quite difficult problem to reproduce and debug too, the original issue here took me quite a few hours to reproduce and find, and likely those will cost too (probably not so much since I know better what's going on).

[GoogleFrog]

Also, it's not just about the selection...

You're missing the point. I'm saying that this PR amounts to embedding a widget in the engine. What you're trying to do here is perfect for lua, yet your solution is to put the behaviour in the engine and make a toggle for it. The fact that you'll want to do a few more things than just deselect the unit only strengthens my point. Lua is great at the following:

• Deselecting units
• Setting units to not be selectable
• Setting control groups
• Filtering dead units out of GetAllUnits (although I really question why lua would be calling GetAllUnits regularly in the first place. I spot checked the BAR repository because your comment worried me).

Maybe you'll think up some "bad behaviour" that lua isn't great at. If so, then there are two options:

1. Extend the API to make lua good at it.
2. Make a modrule to enable that specific behaviour.

There is absolutely no way option 2 is going to be good in this situation. Being killed is an event that can only happen once per unit, and anything you might want to globally toggle with a modrule could be a cheap Spring.SetUnitBla callout in UnitDestroyed. For 2 to be appropriate, you'd need to want something ridiculous like "dead units use QTPFS".

Your hardcode+modrule approach actively makes the engine worse. In many cases the cost might only be in confusing code (why is a widget in the engine??) and bloated options, but the mindset will result in a worse engine in the long run. Unselectable units is a good example.

Imagine that Spring.SetUnitNoSelect doesn't exist. Ok, this is pretty hard to imagine since I checked BAR, and you already have a gadget that makes units unselectable during death animations. At least, it makes the hardcoded list of units that the gadgets claim to have death animation, have death animations. I don't know whether this list is updated sufficiently, but that is a game problem not an engine one. This is a bit of an annoying problem, but you'd be better served by a customparam than a list in a gadget.

Anyway, imagine that Spring.SetUnitNoSelect doesn't exist. I would still be advocating against adding a modrule that makes dying units unselectable, because ZK would already have a gadget that sets a UnitRulesParam on appropriate units, which is then read wherever selection is handled in widget space. But in this hypothetical you would have a good point in saying that lua isn't great at making units unselectable. The appropriate response would be to add Spring.SetUnitNoSelect, especially since adding a hardcoded toggle to make dying units unselectable wouldn't even remove the need to use the UnitRulesParam selection filtering method! Both ZK and BAR have unselectable drones that float around a carrier (at least, BAR copied the ZK gadget for doing so in 2023, so I assume there is at least a modder working on it). So this PR wouldn't even solve the desire for a better way to make units unselectable. All the old selecting filtering code would still exist in these games because they weren't just using it for dying units!

It gets worse, since the PR is no longer just a toggle that does one thing, instead you're going to bundle together a set of "bad behaviours" that the engine should handle. Now consider the following situation.

• A new game developer comes along, peruses the modrules and decides they wouldn't like dead units to be unselectable.
• Later in development, they discover that they would like one exception to one of the hardcoded behaviours you've bundled together. Maybe you decided that projectiles colliding with dying units is a "bad behaviour", yet their game is about fighting giant robots with drawn out death animations, but having the projectiles start piercing the unit at the moment of death looks really stupid.
• I've been in this situation, and the only timely solution is to rip out the engine behaviour and reimplement the bits that you want. It might take years for an engine fix to be integrated, and then what even is the fix? Presumably BAR wants exactly the behaviour implemented by the modrule (and can tweak the modrule relatively quickly if that changes), so do we add another modrule for the specific behaviour that the new game wants? Expand the combined modrule into a table of options?

An example closer to home would be Spring.GetAllUnits. You said that this should exclude dying units, but what happens when a gadget/widget wants dead units as well? Are we going to add Spring.GetAllUnitsEvenDyingOnes? Does the game have to rip out the engine behaviour and reimplement it in lua, as above? Do game devs just have to eat the fact that their widget crashes if enabled while a unit happens to be running its death animation? The last one would be terribly shoddy, and the engine should support devs making non-shoddy games.

In brief:

• This PR doesn't seem to do anything that lua is not already great at.
• If you find something lua isn't great at, then this PR squanders the opportunity to extend the API in ways that would benefit more than the case of dying units.
• Unless the developer of a given game has exactly the same requirements that you assume they have, they are probably going to end up disabling the modrule and doing the lua solution anyway.

Yes, BAR has fundamental issues with its lua. The solution is for BAR to get better at lua. The engine should help with this in generic ways, that are resusable between many games and situations. The engine should not provide a modoption that arbitrarily tweaks bits of the API and engine behaviour just so that a particular game can avoid fixing its lua. Especially when those tweaks amount to a set of hardcoded engine limitations that developers would have to remove when they butt up against them.

[sprunk]

​

[saurtron]

It gets worse, since the PR is no longer just a toggle that does one thing, instead you're going to bundle together a set of "bad behaviours" that the engine should handle. Now consider the following situation.

Well, tbh I don't think bundling more stuff here would be a good idea. I think this one better left just for the selection part that I think is ready already (except for adding the "remove from groups" part also inside same toggle).

• Filtering dead units out of GetAllUnits (although I really question why lua would be calling GetAllUnits regularly in the first place. I spot checked the BAR repository because your comment worried me).

No idea tbh, but I think I seen it at a few places. Let's better leave the GetAllUnits for different discussion since this is already heated enough just with selection lol.

Your hardcode+modrule approach actively makes the engine worse.

Imo it makes it much easier to start with since you won't have to be solving the problem of dead units in selection just for starters.

Also note current implementation is also wrong, since it doesn't remove from selection but it does remove from groups, imo should be both or none. This pr can fix that and then ppl choose what they want.

An example closer to home would be Spring.GetAllUnits. You said that this should exclude dying units, but what happens when a gadget/widget wants dead units as well?

They can use the UnitDestroyed callin.

Yes, BAR has fundamental issues with its lua. The solution is for BAR to get better at lua.

Not sure why you keep flaming at bar all the time when this is engine discussion, imo that's not very nice.

If anything your remark proves is not having a reasonable behavior here just letting ppl shoot themselves in the foot and make it generally more difficult to use. I'd like to see a single game where having a dead selected unit makes sense.

You can create a gadget for this, sure, but what's the point? Just to implement reasonable defaults?

Maybe you'll think up some "bad behaviour" that lua isn't great at. If so, then there are two options:

Maybe having dead units in selection is one such problem, and the one the PR here is supporting tbh.

[saurtron]

There's another problem, and it's that if there was some c++ EventClient listener running before lua, then I think it would be impossible to do a perfect "fix" from lua. I don't think there's one now, but it's not farfetched to think there could be one in the future, so leaving this unsolved could be a problem down the road.

I think you skipped this part, and it's not entirely hypothetical. It's true it can be worked around, and also generally would be nicer to always have any c++ listeners after lua, but in some cases it could be beneficial to have them before if the game chooses to run them earlier.

[GoogleFrog]

I wrote a thorough response with the goal of communicating some ideals of engine design, and improving you as a developer. Length != heat. But I'm not sure how to continue without it getting a bit heated. Here are some dot points.

• If you want to work on making it easier for new game devs, then curate a library of small wupgets that implement simple behaviours. One would implement deselection and non-selectability on units dying. The beauty of this approach is that it would demonstrate features to developers, and let them tweak them as required.
• Your comment "They can use the UnitDestroyed callin." makes no sense if you read the rest of my example. I was talking about wupgets that are initialised in while a unit is dying.
• I'm only "flaming BAR" as much as you are. You said "I think the fact BAR has a lot of problems...", to which I responded to "Yes, BAR has fundamental issues with its lua...". And these comments are relevant because this seems like a BAR-engine issue.
• "Just to implement reasonable defaults?" I already wrote about the pitfalls of implementing optional engine limitations of reasonable defaults.
• "Maybe having dead units in selection is one such problem" I think I've given quite a good argument as to why it isn't. I've yet to hear a counterargument.

Overall I get the impression that you're responding to bits of the post rather than engaging with the post as a whole. I can give up the quotes you took without weakening my argument - they're just examples or different ways to look at the same thing. I'm aware that I've just done a similar thing to your post, but I'm not sure what to do in the face of a list of disconnected problems. But such posts get us into the weeds while the meat of whatever is being said is ignored. So you can ignore the dot points above, but please go and find the answers to your questions within the post I already wrote. You'll find out things such as why it doesn't matter whether you deal with GetAllUnits here or in a later PR.

[sprunk]

On C++ event listeners: they would be supposed to just handle the case correctly regardless of whether Lua did anything or not. If you add one, the issue is sure to show up and be resolved during review, now that we're all acutely aware of it. It's actually easy to handle when making an event client from scratch.

[saurtron]

Well, ppl, first of all I want to remark I'm not here to implement a lot of custom hard coded behaviors into the current codebase, just in case there's any doubts. I would always think thrice before adding modrules or unit/unitdef pars just for that.

Also will say I'm sorry for having all this "flaming".

But all in all I'm not convinced at all by your arguments here, I understand the general concerns, but don't see how those apply to having this here with a modrule (unless being unnecessarily strict about it).

• There's not even a remote use case for dead units in selection.
• Currently the behavior is broken, since units are removed from groups but not from selection, yet, nobody complains about that.
• Even if someone would want to have them there, that's why I'm proposing a probably uneeded modrule to attend your concerns.
  • Could be added when the need actually arises, if ever (could even leave the PR for that ready just in case), otherwise we're just adding cruft to account for highly hypothetical situations.
  • I'm highly sceptical anyone would ever want this, and if they did they would probably approach the same goals without killing the unit first, since kill the unit and then want to do active things is not semantic, it removes meaning from "dead" to the point of making the concept a bit absurd.
• The current implementation for dead units feels more like an afterthought/incomplete solution, imo a dead unit is probably closer to a feature than a unit, and I'm sure the engine and games need to always be very careful about that to not have problems as a consequence.
• The current behavior can indeed cause problems for games. Yes, they can use a gadget to solve, but if they don't it can take some time and pain until they find out. Current behaviour is totally unexpected and unintuitive, and will need to be documented and hope ppl will read it before running into problems.
• Having a modrule isn't even a problem for you, I'm sure you'd be well served by it too, one less gadget is one less file to care about.
• Thinking adding this modrule here is going to compromise the long term vision is far fetched.

Overall I get the impression that you're responding to bits of the post rather than engaging with the post as a whole.

I guess that's normal with discussions and such lengthy messages, I also get this impression but I think it's human nature both ways... no hard feelings.

I can say I read and re-read your messages quite a few times, probably too many already XD.

• Maybe you decided that projectiles colliding with dying units is a "bad behaviour"

I find this a bit of a sophistic affirmation (sorry if this sounds a bit bad, not sure how to say it), projectiles routinely collide with "dead things". Maybe someone decided dead units should be in selection would be more like it, even worse, maybe someone forgot to remove them from there and now we're having this argument.

• then curate a library of small wupgets that implement simple behaviours

With all respect, not going to make a library of workarounds.

• the only timely solution is to rip out the engine behaviour and reimplement the bits that you want

Imo the engine should implement default behaviors in a more modular way that's easier to be enabled/disabled or controlled from lua. I'm all for working on that, I don't think the solution is to rip all default behaviors and move to lua because then ppl who want to have some perf would have no option, I think a better solution is to have a similar structure to the gadget/widget (ie, EventClients), that's been demonstrated to work really well in implementing loads of modular behaviors.

That would allow ppl to utilise those when possible in order to save perf for more interesting things, while also easily allow disabling them to create custom lua implementations. Also that system should allow module configurability without everything going through modrules, as well as probably unit/weapon/...def extensions just like currently there's customparams for lua.

It might take years for an engine fix to be integrated,

What can I say, this problem probably been there for years. I'm all for fixing those problems asap.

Are we going to add Spring.GetAllUnitsEvenDyingOnes?

No, but Spring.GetDeadUnits seems reasonable tbh.

--

All in all, I'm undecided what to do here. I'm definitely tired of having to discuss the same arguments time and time again, as I'm sure you're tired too. I respect both of you guys, since you both have been here much longer than me so I wouldn't even dream of enforcing some vision. This is also taking too many mental cycles that could be dedicated to more productive things, as well as probably generating some distrust. If this would be my first approach at the engine I understand you would be really concerned, but the fact is I already fixed a few years standing bugs and I think demonstrated my commitment to improving the engine and caring about games... I don't mean this means I can now go and break things or enforce a particular vision, but just want to say I didn't come here to break things or make the engine worse.

Still, I find this being blocked on hypotheticals is a bit weird that's why I'm still here, but I hope this resumes my concerns in a way that don't need to be talked about more.

Every day I'm tempted to just close this and make the gadget lol, but tbh don't feel that's the right thing to do.

[saurtron]

Hey @GoogleFrog, @sprunk, since I don't think we're reaching a consensus with this issue, I created an alternative solution. See #1849 and #1853.

The idea is making the behavior consistent by also modifying SetUnitNotSelect to manage the group, and ForcedKillUnit not remove the group. I think this makes the prospect of controlling from lua more straightforward since I find the fact the unit is removed from groups but not selection quite disturbing.

[saurtron]

Closing this one in favour of #1850 and #1853.