Unhardcode autoupgrade
I'm not necessarily in to moving Leonardo of standard rulesets to lua code. Lua scripting skills should not be a requirement for users to modify their rulesets. This doesn't mean we can't have lua API to implement more complex autoupgrade rules in a ruleset that wants to do that instead of using the "stock" effect.
Reply To cazfi
I'm not necessarily in to moving Leonardo of standard rulesets to lua code. Lua scripting skills should not be a requirement for users to modify their rulesets. This doesn't mean we can't have lua API to implement more complex autoupgrade rules in a ruleset that wants to do that instead of using the "stock" effect.
I think similarily. I just put into default.lua a callback that does the default procedure based merely on the effect value - we still need the effect for AI and autotext, I'm not going to change it. And whoever wants another behaviour, may override the callback.
Done as said before. Description comes in commit message, no changes to existing rulesets needed (unless we make some clarification somewhere of how to override it).
doc/README.effects needs to be updated with the information that the Upgrade_Unit effect now does, well, nothing beyond what the ruleset does via lua scripting (and making the AI think its use scales with upgradable units).
Which I have a bit of an issue with. With this patch, on a game level, the Upgrade_Unit effect is literally no different from the user effects. I'm not sure I like the idea of a special, named user effect that's given different treatment by the AI, without actually being different in any material way.
Reply To alienvalkyrie
I'm not sure I like the idea of a special, named user effect that's given different treatment by the AI, without actually being different in any material way.
That's what I'm thinking too. The effect should still be handled by the C-code. Any ruleset that wants to handle upgrade via lua-script just should leave the effect value to zero.
Reply To cazfi
Reply To alienvalkyrie
I'm not sure I like the idea of a special, named user effect that's given different treatment by the AI, without actually being different in any material way.
That's what I'm thinking too. The effect should still be handled by the C-code. Any ruleset that wants to handle upgrade via lua-script just should leave the effect value to zero.
Actually, I based my solution on existing "Inspire_Partisans" effect that is also for long handled only by Lua callback, helptext and AI. You can in theory handle the effect in any other way or call the API function from any other callback but it's up to you to keep the necessary semantics. And currently we have no tools to give ideas to the AI when and how much an effect is desired unless some hardcoded values are given to it; also, why not to supply an autohelp that most commonly is true and in other cases may be turned off.
"Upgrade_Unit", "Inspire_Partisans" and some other effects don't modify how something happens in the game (as e.g. "Combat_Rounds" do), they are event bonuses that just at certain events bring something else. They naturally may be realized in a signal-callback mechanism and in fact they are in C code, moving the callback to Lua just gives much bigger variability to the behaviour and puts that event into a common way of processing.
Reply To ihnatus
Actually, I based my solution on existing "Inspire_Partisans" effect that is also for long handled only by Lua callback, helptext and AI.
The (albeit minute) difference being that Inspire_Partisans is still handled explicitly in city:inspire_partisans() (with additional checks and special targets to evaluate against). But yeah – I can't say I'm entirely happy with how Inspire_Partisans currently works either.
And currently we have no tools to give ideas to the AI when and how much an effect is desired unless some hardcoded values are given to it; also, why not to supply an autohelp that most commonly is true and in other cases may be turned off.
You're absolutely right, some general mechanism to add AI hints and helptext to user effects could certainly help with this problem.
moving the callback to Lua just gives much bigger variability to the behaviour and puts that event into a common way of processing.
It's a tradeoff though – it also means we cannot make any assumptions about how it works anymore. Long-term, I'd hope that we'll be able to un-script all of these things, turning them into a part of the ruleset that can be reasoned about (by sanity checks, autohelp and AI).
To be clear, I'm not necessarily opposed to moving autoupgrade to Lua script for now – I just
I didn't remember about Inspire_Partisans. I don't particularly like that one either, but it's true that it's something that we already have.
Reply To cazfi
I didn't remember about Inspire_Partisans. I don't particularly like that one either, but it's true that it's something that we already have.
Also, we have Lua-coded enter/frighten hut effects...
Hmm.. this is not currently targeted to S3_1-d3f, but it should if we are still going to have it in 3.1 at all. So how do we proceed with this to unblock S3_1-d3f?
Reply To cazfi
Hmm.. this is not currently targeted to S3_1-d3f, but it should if we are still going to have it in 3.1 at all.
Setting that target, even if only temporarily, so that queries about d3f blockers will list this ticket.
Reply To alienvalkyrie
To be clear, I'm not necessarily opposed to moving autoupgrade to Lua script for now – I just a. think it's important to have a plan for how we're gonna do this more cleanly later, and b. am worried that even if we have a plan, this might turn into one of those "temporary" solutions that will be lying around for eighteen years before someone notices there's a better way now.
I don't think anybody is working on such a plan now? Does that alone mean that we're rejecting this from S3_1? (I'm not saying that it's the only thing blocking this. It might just be something that renders further considerations irrelevant.)
Reply To cazfi
Reply To alienvalkyrie
To be clear, I'm not necessarily opposed to moving autoupgrade to Lua script for now – I just
a. think it's important to have a plan for how we're gonna do this more cleanly later, and
b. am worried that even if we have a plan, this might turn into one of those "temporary" solutions that will be lying around for eighteen years before someone notices there's a better way now.
I don't think anybody is working on such a plan now? Does that alone mean that we're rejecting this from S3_1? (I'm not saying that it's the only thing blocking this. It might just be something that renders further considerations irrelevant.)
The hint of a plan I have would be to make User Effects variable and definable (like e.g. user-defined terrain flags), and then maybe allow attaching AI hints to those. There's obviously no way that's still ending up in S3_1, but if we solidify that as a plan, we might be able to justify merging this patch now already (although it would still feel a bit like putting the cart before the horse, half-unhardcoding something before the potential to fully unhardcode it exists).
One relevant question would be: Is there a current, acute need from ruleset authors to customize the upgrade code – in a context where the AI still has to care about it? Because that would be a circumstance requiring what this patch does.
Reply To alienvalkyrie
One relevant question would be: Is there a current, acute need from ruleset authors to customize the upgrade code – in a context where the AI still has to care about it? Because that would be a circumstance requiring what this patch does.
My idea that mostly has inspired this group of tickets was to make it possible to use different buildings for upgrading different unit classes (it's only one way you can improve Leonardo, maybe it's also natural that it works only in native cities etc.). AI should have a hint that such buildings have positive effects, that without much code rewriting is done by supplying the old Leonardo effect but doing a bit different thing on the gameboard. Also, behind it, I had a vision that some most randomly defined game bonuses, the "gimmicks" like Leonardo or Internet effects, should be written in a script engine to be easily replaced by some other gimmick a ruleset author may invent; but likely you don't see everything like me.
I had some ideas about binding sophisticated ruleset features to AI tactics. My first thought was defining pseudo-effects and pseudo-requirements that don't affect the game but AIs think they do, but this is obviously a kludge. Another thought was special group of effects like "AI_Strategical_Action_Success_Pml" or "AI_City_Building_Desire" (you talk about something like this, yes?), but the system seems to be overtly bound to specific AI engine, rather fuzzy in understanding by users, and we'll hardly be ever able to put all into the language of effects and requirements. Yet another way is governing AI from scripts, having a class "AI" with some basic methods and properties and its descendants like "Classic_AI" that have their own ones, but it's still a lot of work having no known worker.
Reply To ihnatus
My idea that mostly has inspired this group of tickets was to make it possible to use different buildings for upgrading different unit classes (it's only one way you can improve Leonardo, maybe it's also natural that it works only in native cities etc.). AI should have a hint that such buildings have positive effects, that without much code rewriting is done by supplying the old Leonardo effect but doing a bit different thing on the gameboard.
I actually had my own idea for that kind of thing a while back (involving the Upgrade_Unit effect being evaluated at unit level, rather than at player level), but I didn't act on it, since it becomes moot when unhardcoded anyway.
the "gimmicks" like Leonardo or Internet effects, should be written in a script engine to be easily replaced by some other gimmick a ruleset author may invent
Oh yeah, long-term, absolutely; I'm just not convinced the short-term advantages of having half a solution now outweigh the long-term difficulties from having half a solution in the way when trying to put in a new, full solution.
I had some ideas about binding sophisticated ruleset features to AI tactics. (...)
We're topic drifting a bit here (I'll open a separate AI hinting metaticket later when I'm less busy), but I feel like a bespoke solution might be called for here; I was thinking attaching special hints to user-defined stuff like unit type/class and terrain flags, and (when we add them) custom user actions, custom effect types etc. That might strike the balance between expressiveness and ease-of use for ruleset authors.
First of all: Time just before d3f might be the most stressful part of our major version cycle. The last blocker issues that we've been unable to resolve so far (sometimes over many years), i.e., the toughest ones, need to be coded, and decisions like one in this ticket need to be done. I'm rather torn with this one.
Reply To alienvalkyrie
One relevant question would be: Is there a current, acute need from ruleset authors to customize the upgrade code – in a context where the AI still has to care about it? Because that would be a circumstance requiring what this patch does.
Also; is this patch alone enough for such needs? Because if something more, that is no longer going in before d3f, would be required, then there's no point in this patch either/alone in S3_1.
In general, judging just from the level where the discussion goes, not caring about the actual content, it sounds more like an attempt to decide if this should go to master development branch, than of an attempt to decide if this should go to a stable branch.
Reply To cazfi
Reply To alienvalkyrie
One relevant question would be: Is there a current, acute need from ruleset authors to customize the upgrade code – in a context where the AI still has to care about it? Because that would be a circumstance requiring what this patch does.
Also; is this patch alone enough for such needs?
As far as I can tell, yes, so long as it doesn't require multiple effects with different AI consideration.
it sounds more like an attempt to decide if this should go to master development branch, than of an attempt to decide if this should go to a stable branch.
Good point. @ihnatus: would you be fine with pushing this back to S3_2; both to remove it from the S3_1-d3f stress, and to give us more time to figure out what exactly to do with this issue?
Reply To alienvalkyrie
We're topic drifting a bit here (I'll open a separate AI hinting metaticket later when I'm less busy)
~> #44355
Reply To alienvalkyrie
Good point. @ihnatus: would you be fine with pushing this back to S3_2; both to remove it from the S3_1-d3f stress, and to give us more time to figure out what exactly to do with this issue?
For any issue postponed, I recommend that you still try to keep it active on master.
Very often we've seen something postponed from one version to activate only when it's blocking the next version, with a high risk of getting postponed again. Repeat.
With S3_1 d3f I've tried to handle immediately as many of those blocking issues as possible, even when they have been postponed to 3.2. Though in a number of cases the order has been that I've first implemented something for S3_1 d3f, and *then* decided that the implementation is too risky to include to S3_1 and thus postponed / pushed to master only.
Reply To alienvalkyrie
it sounds more like an attempt to decide if this should go to master development branch, than of an attempt to decide if this should go to a stable branch.
Good point. @ihnatus: would you be fine with pushing this back to S3_2; both to remove it from the S3_1-d3f stress, and to give us more time to figure out what exactly to do with this issue?
Well, I'll be fine. Likely, the questions risen by this patch are beyound my level of involvement in the project. It well may be that we just need to invent more instruments to do the desired thing good.
I actually had my own idea for that kind of thing a while back (involving the Upgrade_Unit effect being evaluated at unit level, rather than at player level), but I didn't act on it, since it becomes moot when unhardcoded anyway.
I also started from this idea but there are problems that don't allow doing it that simple, see this topic.
I now hacked together #44711 as a one possible solution to the issue here (made it separate ticket - didn't want to have different threads in this one ticket).
- "Upgrade_Unit" effect would still work exactly the same as before
- One could use user effect instead of "Upgrade_Unit" for implementing autoupgrade in lua script
- One could set AI to evaluate that user effect like "Upgrade_Unit"
We should probably add more user effect slots for this - currently we have only four in total, and upgrade from older ruleset versions can already reserve them all.
Reply To cazfi
Reply To cazfi
I now hacked together #44711 as a one possible solution to the issue here (made it separate ticket - didn't want to have different threads in this one ticket).
Does that approach meet your needs?
Will close this ticket soon, if no further comments about still needing something here that was not delivered by #44711.
If you feel like closing it you probably are the person to do it...
Reply To cazfi
Reply To cazfi
Reply To cazfi
I now hacked together #44711 as a one possible solution to the issue here (made it separate ticket - didn't want to have different threads in this one ticket).
Does that approach meet your needs?
Will close this ticket soon, if no further comments about still needing something here that was not delivered by #44711.
No objections in two years -> closing.
Autoupgrade (Leonardo effect) is a gimmick bonus, like hut findings, but it is now hardcoded. The result is, we can't have any good way for adding flexibility to it (e.g., upgrade only units in domestic borders, or have wonders that upgrade different classes separately). So, the logical solution is: write autoupgrade code into a Lua callback in default.lua, and the effect "Upgrade_Unit" will just be used by AI and autohelp as it is, and the default callback looks at it but some other one might not. To do it, we must develop in separate tickets: