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

Add autogroups on UnitCreated #3740

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hihoman23
Copy link
Member

Work done

The autogroup widget adds the control groups on unit creation on immediate mode.

closes #3449

@badosu
Copy link
Contributor

badosu commented Sep 17, 2024

This is a very problematic piece of code, I am unable to review or test this properly, I just want to remark to thoroughly test the autogroup widget use cases (immediate mode on/off, built from factory with order/not, rezzed, given, captured, built in place by mobile builder (e.g. consul) etc)

@WatchTheFort
Copy link
Member

@hihoman23 Has this been tested in line with badosu's comment?

@hihoman23
Copy link
Member Author

No, I sadly haven’t gotten to it.

@badosu
Copy link
Contributor

badosu commented Oct 11, 2024

Yeah, don't mean to block others work unnecessarily.

It's just too often we see contributions in widgets with lots of features creating regressions up to a point they get unusable later and very hard to fix back to original state (e.g. mex snap, that I had to fix like 4 times over again until I simply gave up).

@badosu
Copy link
Contributor

badosu commented Oct 11, 2024

Normally I don't ask this for simple changes (even though this looks like a simple one), just widgets which feature a lot of different modes and use cases. Often one gets their own use case 'fixed' while breaking some other untested one.

One option is to ask players to playtest it (I often do and ask so, pinging and asking the testers discord group for assistance) as a custom widget and, if after a few weeks no issues are found, then at least we have some 'good enough' testing in place (this unfortunately only tests the 'popular' use cases, but it also can find a lot of smaller bugs youd never test). The other option is for the author or someone else to thoroughly go through all options and expected scenarios to make sure all is good.

This seems like something one can test on their own, the tricky part (sometimes) is knowing all expected scenarios autogroup should satisfy.

@sprunk
Copy link
Collaborator

sprunk commented Oct 11, 2024

Could have testing instructions in the comments, listing cases.

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.

Add additional option for "Autogroup immediate mode" that adds a unit into autogroup while it's being built
4 participants