Derive unitDef.category from Unitdefs

kroIya commented 3 months ago

Work done

This PR derives most target categories in post to represent what the unit actually is.

Currently the categories are populated manually, which leads to many inconsistencies. The export of current categories can be seen here: https://docs.google.com/spreadsheets/d/1RLr-YlIFInJA2iCrBLQZO8xLufFhD6XgbQ0ZfZmC-Ag/edit?usp=sharing

List of derived categories: ALL MOBILE NOTMOBILE WEAPON NOWEAPON VTOL NOTAIR HOVER NOTHOVER SHIP NOTSHIP NOTSUB CANBEUW UNDERWATER SURFACE MINE COMMANDER EMPABLE

List of manual categories: OBJECT T4AIR LIGHTAIRSCOUT GROUNDSCOUT RAPTOR

List of deprecated categories: BOT - unused, can not see a possible use for it; TANK - unused, can not see a possible use for it; PHIB - all possible cases covered by HOVER CANBEUW UNDERWATER; NOTLAND - currently seems to mean "anything that isn't a land unit", which includes land buildings. I believe all current uses of it are in error (e.g. mobile t2 arty having buildings as badcategory). Partially replaced by a new category NOTMOBILE; SPACE - reserved for satellites, not in the base game, not sure if those are needed going forward.

Currently works as follows: Remember the manual categories => Erase all categories => Add the manual categories back in => Add the derived categories, ignoring those listed in unitDef.exemptcategory

After the unitdefs are cleaned up to not contain erroneous categories (e.g. CANBEUW on armthor), will work: Leave the manual categories in => Add the derived categories, ignoring those listed in unitDef.exemptcategory

Setup

No additional setup required.

Since this deals with unitdefs exclusively, it can be tested as a tweakdef on the live release: bG9jYWwgZnVuY3Rpb24gbWFudWFsQ2F0ZWdvcnkodW5pdERlZiwgY2F0ZWdvcnlTdHJpbmcpIHJldHVybiBzdHJpbmcuZmluZCh1bml0RGVmLmNhdGVnb3J5LCBjYXRlZ29yeVN0cmluZykgZW5kCglsb2NhbCBmdW5jdGlvbiBhcHBlbmQodW5pdERlZiwgYXBwZW5kU3RyaW5nKSBkbyB1bml0RGVmLmNhdGVnb3J5ID0gdW5pdERlZi5jYXRlZ29yeS4uYXBwZW5kU3RyaW5nIGVuZCBlbmQKCQoJLS0gdW5pdERlZi5tb3ZlbWVudGNsYXNzIGxpc3RzCglsb2NhbCBob3Zlckxpc3QgPSB7CgkJSE9WRVIyID0gdHJ1ZSwKCQlIT1ZFUjMgPSB0cnVlLAoJCUhIT1ZFUjQgPSB0cnVlLAoJCUhPVkVSNSA9IHRydWUKCX0KCQoJbG9jYWwgc2hpcExpc3QgPSB7CgkJQk9BVDMgPSB0cnVlLAoJCUJPQVQ0ID0gdHJ1ZSwKCQlCT0FUNSA9IHRydWUsCgkJQk9BVDggPSB0cnVlLAoJCUVQSUNTSElQID0gdHJ1ZQoJfQoJCglsb2NhbCBzdWJMaXN0ID0gewoJCVVCT0FUNCA9IHRydWUsCgkJRVBJQ1NVQk1BUklORSA9IHRydWUKCX0KCQoJbG9jYWwgYW1waGliTGlzdCA9IHsKCQlWQk9UNSA9IHRydWUsCgkJQ09NTUFOREVSQk9UID0gdHJ1ZSwKCQlTQ0FWQ09NTUFOREVSQk9UID0gdHJ1ZSwKCQlBVEFOSzMgPSB0cnVlLAoJCUFCT1QyID0gdHJ1ZSwKCQlIQUJPVDQgPSB0cnVlLAoJCUFCT1RCT01CMiA9IHRydWUsCgkJRVBJQ0JPVCA9IHRydWUsCgkJRVBJQ0FMTFRFUlJBSU4gPSB0cnVlCgl9CgoJbG9jYWwgY29tbWFuZGVyTGlzdCA9IHsKCQlDT01NQU5ERVJCT1QgPSB0cnVlLAoJCVNDQVZDT01NQU5ERVJCT1QgPSB0cnVlCgl9CgkKCWxvY2FsIGNhdGVnb3JpZXMgPSB7fQoKCS0tIE1hbnVhbCBjYXRlZ29yaWVzOiBPQkpFQ1QgVDRBSVIgTElHSFRBSVJTQ09VVCBHUk9VTkRTQ09VVCBSQVBUT1IKCS0tIERlcHJlY2F0ZWQgY2FyZWdvcmllczogQk9UIFRBTksgUEhJQiBOT1RMQU5EIFNQQUNFCgkKCWNhdGVnb3JpZXNbIkFMTCJdID0gZnVuY3Rpb24oKSByZXR1cm4gdHJ1ZSBlbmQKCWNhdGVnb3JpZXNbIk1PQklMRSJdID0gZnVuY3Rpb24odW5pdERlZikgcmV0dXJuIHVuaXREZWYuc3BlZWQgYW5kIHVuaXREZWYuc3BlZWQgPiAwIGVuZAoJY2F0ZWdvcmllc1siTk9UTU9CSUxFIl0gPSBmdW5jdGlvbih1bml0RGVmKSByZXR1cm4gbm90IGNhdGVnb3JpZXMuTU9CSUxFKHVuaXREZWYpIGVuZAoJY2F0ZWdvcmllc1siV0VBUE9OIl0gPSBmdW5jdGlvbih1bml0RGVmKSByZXR1cm4gdW5pdERlZi53ZWFwb25kZWZzIH49IG5pbCBlbmQKCWNhdGVnb3JpZXNbIk5PV0VBUE9OIl0gPSBmdW5jdGlvbih1bml0RGVmKSByZXR1cm4gbm90IGNhdGVnb3JpZXMuV0VBUE9OKHVuaXREZWYpIGVuZAoJY2F0ZWdvcmllc1siVlRPTCJdID0gZnVuY3Rpb24odW5pdERlZikgcmV0dXJuIHVuaXREZWYuY2FuZmx5ID09IHRydWUgZW5kCgljYXRlZ29yaWVzWyJOT1RBSVIiXSA9IGZ1bmN0aW9uKHVuaXREZWYpIHJldHVybiBub3QgY2F0ZWdvcmllcy5WVE9MKHVuaXREZWYpIGVuZAoJY2F0ZWdvcmllc1siSE9WRVIiXSA9IGZ1bmN0aW9uKHVuaXREZWYpIHJldHVybiBob3Zlckxpc3RbdW5pdERlZi5tb3ZlbWVudGNsYXNzXSBhbmQgKHVuaXREZWYubWF4d2F0ZXJkZXB0aCA9PSBuaWwgb3IgdW5pdERlZi5tYXh3YXRlcmRlcHRoIDwgMSkgZW5kIC0tIGNvbnZlcnRpYmxlIHRhbmsvYm9hdHMgaGF2ZSBtYXh3YXRlcmRlcHRoCgljYXRlZ29yaWVzWyJOT1RIT1ZFUiJdID0gZnVuY3Rpb24odW5pdERlZikgcmV0dXJuIG5vdCBjYXRlZ29yaWVzLkhPVkVSKHVuaXREZWYpIGVuZAoJY2F0ZWdvcmllc1siU0hJUCJdID0gZnVuY3Rpb24odW5pdERlZikgcmV0dXJuIHNoaXBMaXN0W3VuaXREZWYubW92ZW1lbnRjbGFzc10gb3IgKGhvdmVyTGlzdFt1bml0RGVmLm1vdmVtZW50Y2xhc3NdIGFuZCB1bml0RGVmLm1heHdhdGVyZGVwdGggYW5kIHVuaXREZWYubWF4d2F0ZXJkZXB0aCA-PTEpIGVuZAoJY2F0ZWdvcmllc1siTk9UU0hJUCJdID0gZnVuY3Rpb24odW5pdERlZikgcmV0dXJuIG5vdCBjYXRlZ29yaWVzLlNISVAodW5pdERlZikgZW5kCgljYXRlZ29yaWVzWyJOT1RTVUIiXSA9IGZ1bmN0aW9uKHVuaXREZWYpIHJldHVybiBub3Qgc3ViTGlzdFt1bml0RGVmLm1vdmVtZW50Y2xhc3NdIGVuZAoJY2F0ZWdvcmllc1siQ0FOQkVVVyJdID0gZnVuY3Rpb24odW5pdERlZikgcmV0dXJuIGFtcGhpYkxpc3RbdW5pdERlZi5tb3ZlbWVudGNsYXNzXSBlbmQKCWNhdGVnb3JpZXNbIlVOREVSV0FURVIiXSA9IGZ1bmN0aW9uKHVuaXREZWYpIHJldHVybiAodW5pdERlZi5taW53YXRlcmRlcHRoIGFuZCB1bml0RGVmLndhdGVybGluZSA9PSBuaWwpIG9yICh1bml0RGVmLm1pbndhdGVyZGVwdGggYW5kIHVuaXREZWYud2F0ZXJsaW5lID4gdW5pdERlZi5taW53YXRlcmRlcHRoKSBlbmQKCWNhdGVnb3JpZXNbIlNVUkZBQ0UiXSA9IGZ1bmN0aW9uKHVuaXREZWYpIHJldHVybiBub3QgY2F0ZWdvcmllcy5VTkRFUldBVEVSKHVuaXREZWYpIGFuZCBub3QgY2F0ZWdvcmllcy5WVE9MKHVuaXREZWYpIGVuZAoJY2F0ZWdvcmllc1siTUlORSJdID0gZnVuY3Rpb24odW5pdERlZikgcmV0dXJuIHVuaXREZWYud2VhcG9uZGVmcyBhbmQgdW5pdERlZi53ZWFwb25kZWZzLm1pbmVyYW5nZSBlbmQKCWNhdGVnb3JpZXNbIkNPTU1BTkRFUiJdID0gZnVuY3Rpb24odW5pdERlZikgcmV0dXJuIGNvbW1hbmRlckxpc3RbdW5pdERlZi5tb3ZlbWVudGNsYXNzXSBlbmQKCWNhdGVnb3JpZXNbIkVNUEFCTEUiXSA9IGZ1bmN0aW9uKHVuaXREZWYpIHJldHVybiBjYXRlZ29yaWVzLlNVUkZBQ0UodW5pdERlZikgYW5kIHVuaXREZWYuY3VzdG9tcGFyYW1zIGFuZCB1bml0RGVmLmN1c3RvbXBhcmFtcy5wYXJhbHl6ZW11bHRpcGxpZXIgfj0gMCBlbmQKCQoJCglmb3IgbmFtZSwgdW5pdERlZiBpbiBwYWlycyhVbml0RGVmcykgZG8KCQlpZiBzdHJpbmcuZmluZCh1bml0RGVmLmNhdGVnb3J5LCAiT0JKRUNUIikgdGhlbiAtLSBvYmplY3RzIHNob3VsZCBub3QgYmUgdGFyZ2V0YWJsZSBhbmQgdGhlcmVmb3JlIGFyZSBub3QgYXNzaWduZWQgYW55IG90aGVyIGNhdGVnb3J5CgkJZWxzZQoJCgkJCS0tIHRlbXJvcmFyeSBjb2RlLCBwZW5kaW5nIHVuaXRkZWYgY2xlYW51cAoJCQlsb2NhbCBpc1Q0QUlSCgkJCWxvY2FsIGlzTElHSFRBSVJTQ09VVAoJCQlsb2NhbCBpc0dST1VORFNDT1VUCgkJCWxvY2FsIGlzUkFQVE9SCgkJCWlmIG1hbnVhbENhdGVnb3J5KHVuaXREZWYsICJUNEFJUiIpIHRoZW4gaXNUNEFJUiA9IHRydWUgZW5kCgkJCWlmIG1hbnVhbENhdGVnb3J5KHVuaXREZWYsICJMSUdIVEFJUlNDT1VUIikgdGhlbiBpc0xJR0hUQUlSU0NPVVQgPSB0cnVlIGVuZAoJCQlpZiBtYW51YWxDYXRlZ29yeSh1bml0RGVmLCAiR1JPVU5EU0NPVVQiKSB0aGVuIGlzR1JPVU5EU0NPVVQgPSB0cnVlIGVuZAoJCQlpZiBtYW51YWxDYXRlZ29yeSh1bml0RGVmLCAiUkFQVE9SIikgdGhlbiBpc1JBUFRPUiA9IHRydWUgZW5kCgkJCXVuaXREZWYuY2F0ZWdvcnkgPSAiIgoJCQlpZiBpc1Q0QUlSID09IHRydWUgdGhlbiBhcHBlbmQodW5pdERlZiwgIiBUNEFJUiIpIGVuZAoJCQlpZiBpc0xJR0hUQUlSU0NPVVQgPT0gdHJ1ZSB0aGVuIGFwcGVuZCh1bml0RGVmLCAiIExJR0hUQUlSU0NPVVQiKSBlbmQKCQkJaWYgaXNHUk9VTkRTQ09VVCA9PSB0cnVlIHRoZW4gYXBwZW5kKHVuaXREZWYsICIgR1JPVU5EU0NPVVQiKSBlbmQKCQkJaWYgaXNSQVBUT1IgPT0gdHJ1ZSB0aGVuIGFwcGVuZCh1bml0RGVmLCAiIGlzUkFQVE9SIikgZW5kCgkJCWlmIG5hbWUgPT0gImFybW1leCIgb3IgbmFtZSA9PSAiY29ybWV4IiBvciBuYW1lID09ICJsZWdtZXgiIG9yIG5hbWUgPT0gImxlZ21leHQxNSIgdGhlbiBhcHBlbmQodW5pdERlZiwgIiBDQU5CRVVXIikgZW5kCgkJCS0tIGVuZCBvZiB0ZW1wb3JhcnkgY29kZQoJCgkJCWZvciBjYXRlZ29yeU5hbWUsIGNvbmRpdGlvbiBpbiBwYWlycyhjYXRlZ29yaWVzKSBkbwoJCQkJaWYgdW5pdERlZi5leGVtcHRjYXRlZ29yeSA9PSBuaWwgb3Igbm90IHN0cmluZy5maW5kKHVuaXREZWYuZXhlbXB0Y2F0ZWdvcnksIGNhdGVnb3J5TmFtZSkgdGhlbgoJCQkJCWlmIGNvbmRpdGlvbih1bml0RGVmKSB0aGVuCgkJCQkJCWFwcGVuZCh1bml0RGVmLCAiICIgLi4gY2F0ZWdvcnlOYW1lKQoJCQkJCWVuZAoJCQkJZW5kCgkJCWVuZAoJCWVuZAoJZW5k

Test steps

[x] Test that the updated game loads into a Skirmish
[x] Test that the categories are populated by the script
[x] Test that every category works and is applied (or not applied) when appropriate, across a sample of units
[x] Test for anomalies in live gameplay

sprunk commented 3 months ago

Probably needs to cull most existing categories and add exemptions. Right now I think it will just cram all those cats into the existing string that already has them

kroIya commented 3 months ago

Probably needs to cull most existing categories and add exemptions. Right now I think it will just cram all those cats into the existing string that already has them

Already doing this with unitDef.category = "" on line 734.

As stated above, the current order of operations: Remember the manual categories => Erase all categories => Add the manual categories back in => Add the derived categories, ignoring those listed in unitDef.exemptcategory

Once the unitdefs are cleaned manually to not contain any automatic (or otherwise wrong) categories, it will look like this: Skip the manual categories already in => Add the derived categories, ignoring those listed in unitDef.exemptcategory

sprunk commented 3 months ago

Makes sense. A potential worry is that until you do the cleanup, the category string in unit defs will be in a confusing state where changing manual categories works but the automated ones won't. But if the cleanup comes quick enough it's probably fine

kroIya commented 2 months ago

Tested with the tweakdefs in live multiplayer games. Nothing unusual spotted.

beyond-all-reason / Beyond-All-Reason