FreeOrion

Forums for the FreeOrion project
It is currently Wed Jun 20, 2018 10:37 pm

All times are UTC




Post new topic Reply to topic  [ 13 posts ] 
Author Message
PostPosted: Wed Feb 21, 2018 8:46 pm 
Offline
AI Lead, Programmer
User avatar

Joined: Sat Sep 22, 2012 6:25 pm
Posts: 4648
This post is about an issue with both scripting and programming aspects, so I'm putting it here.

When adding some extra effects trace logging to try tracking down this issue with wrong troop regen, I noticed a much more minor issue, but being frustrated with the troops issue, and wanting to get at least something more productive accomplished, I am posting on this more minor issue.

What caught my eye was some interesting looking logs entries, like the following (which is slightly redacted and with some additional newlines added for clarity)
Quote:
[trace] effects : Universe.cpp:1103 : StoreTargetsAndCausesOfEffectsGroups: effects_group: LARGE_PLANET_LABEL
....specific_cause: SHP_GAL_EXPLO sources: Invincible Nipiruk I (9156) )
[trace] effects : Universe.cpp:1078 : Generated new target set, for Condition: that belongs to source's owner empire and that is a Large planet
....targets: (Scanning Facility (31366) Automated History Analyser (23020) Imperial Palace (9380) Cultural Archives (9366) Orbital Drydock (9352) Basic Shipyard (9338) Invincible Nipiruk I (9156) )


The fact that all these buildings are being listed as targets for a condition described as "that is a Large planet" caught my eye. They do properly match the current script, as a very minor adjustment I think we should edit the planet-size description to me more clear, as our Effects Tutorial is, to be "that is, or is contained by, a X planet"

I also pondered the somewhat more significant issue of whether we should adjust the scripts for such things, in this case the pertinent part is
Code:
            scope = And [
                OwnedBy empire = Source.Owner
                Planet size = Large
            ]
to be first add a specific Planet condition and rule out all the buildings here
Code:
            scope = And [
                OwnedBy empire = Source.Owner
                Planet
                Planet size = Large
            ]

I think that the cpu efficiency of both of determining the scope and of ultimately executing the Effects against the smaller set of targets would be improved enough to be worth the extra bit of not-obviously-not-redundancy in the scripting. It would also improve the conciseness/clarity of Effects trace logging when trying debug things as I am now. I wanted to double check if others shared my expectation of efficiency and how it balances against script conciseness.

As a minor point also, I noted in SHP_GAL_EXPLO that most of the EffecsGroups had
Code:
activation = Planet
which may have been a holdover from some other placement, but is just a waste of space & cpu here, correct? The Source for a tech will virtually always be a planet, and even if the empire were without planets it's not like we'd want these effects to no longer apply (or care). So unless someone points out I'm mistaken there I might get around to cleaning up that as well.

_________________
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0


Top
 Profile  
 
PostPosted: Wed Feb 21, 2018 8:53 pm 
Offline
Programming, Design, Admin
User avatar

Joined: Wed Oct 08, 2003 1:33 am
Posts: 12223
Location: Munich
Dilvish wrote:
As a minor point also, I noted in SHP_GAL_EXPLO that most of the EffecsGroups had
Code:
activation = Planet
which may have been a holdover from some other placement, but is just a waste of space & cpu here, correct? The Source for a tech will virtually always be a planet, and even if the empire were without planets it's not like we'd want these effects to no longer apply (or care). So unless someone points out I'm mistaken there I might get around to cleaning up that as well.
Testing if a single object, which is more-or-less guaranteed to be a planet, is a planet is probably very slightly slower. It could be just omitted, but don't make removing them a priority.

For the is vs is-on-a condition ambiguity, is it ever intentionally used in the sense of is-on-a? I wonder if it should be made to do what it appears to, and not have the implicit contain feature for those types of conditions...


Top
 Profile  
 
PostPosted: Thu Feb 22, 2018 2:52 pm 
Offline
Psionic Snowflake

Joined: Tue Sep 30, 2014 10:01 am
Posts: 485
No matter how many years i script FOCS, i'm always baffled about what i dont know...

Dilvish wrote:
Code:
            scope = And [
                OwnedBy empire = Source.Owner
                Planet
                Planet size = Large
            ]

I think if "Planet size = Large" has the isA-or-containedBy semantics, also "Planet" should have it. So this Planet condition should be redundant.

How about having for both the isA-or-containdedBy semantic and add an explicit isA condition e.g. like this:
Code:
            scope = And [
                OwnedBy empire = Source.Owner
                Object isA = Planet size = Large  // this wont match objects contained by a large Planet
            ]


Quote:
The Source for a tech will virtually always be a planet, and even if the empire were without planets it's not like we'd want these effects to no longer apply (or care). So unless someone points out I'm mistaken there I might get around to cleaning up that as well.

Didnt know that either. Shouldnd the source be or include the empire?
What I would love that tech source would actually match all the empire's objects and the Empire itself. That would give the most scripting flexibility. The current techs would have to add an is-the-Empire condition at the start (e.g. Object isA = Empire). And the first thing to check a match for the condition would be the techs owning empire.

_________________
Any code or patches in anything posted here is released under the CC and GPL licences in use for the FO project.


Top
 Profile  
 
PostPosted: Thu Feb 22, 2018 5:13 pm 
Offline
AI Lead, Programmer
User avatar

Joined: Sat Sep 22, 2012 6:25 pm
Posts: 4648
Ophiuchus wrote:
I think if "Planet size = Large" has the isA-or-containedBy semantics, also "Planet" should have it. So this Planet condition should be redundant. How about having for both the isA-or-containdedBy semantic and add an explicit isA condition e.g. like this:
We already have a syntax for describing these things, and an entire section in our Effects Details page devoted to clarifying this aspect of it. I don't think it is worth changing our parser and a zillion scripts to change the syntax. It would take some good sed-fu for that syntax change to not be a nightmare.

Quote:
Didnt know that either. Shouldnd the source be or include the empire?
The Source is always a single object, which is pretty important for the efficiency of our code. I have now edited the Scripting Details section that discusses empire Source and the Techs section to state that this is also used for all EffectsGroups in Techs.

Quote:
What I would love that tech source would actually match all the empire's objects and the Empire itself. That would give the most scripting flexibility. The current techs would have to add an is-the-Empire condition at the start (e.g. Object isA = Empire). And the first thing to check a match for the condition would be the techs owning empire.
Empires are not Objects, so that is simply unworkable (but I acknowledge it can seem a bit tricky at first when scripting, and took me a while to get used to). If you want to match all objects owned by an empire, then use OwnedBy.

I suspect that you've gotten a lot of scripting experience under your belt since the last time you read through the scripting details page, and that if you give it a thorough reading now you are better prepared to really learn all that it covers.

_________________
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0


Top
 Profile  
 
PostPosted: Thu Feb 22, 2018 8:59 pm 
Offline
Psionic Snowflake

Joined: Tue Sep 30, 2014 10:01 am
Posts: 485
Dilvish wrote:
Ophiuchus wrote:
Didnt know that either. Shouldnd the source be or include the empire?
The Source is always a single object, which is pretty important for the efficiency of our code.

Thanks for the documentation update. I should have phrased that suggestion better.

Dilvish wrote:
Ophiuchus wrote:
What I would love that tech source would actually match all the empire's objects and the Empire itself. That would give the most scripting flexibility. The current techs would have to add an is-the-Empire condition at the start (e.g. Object isA = Empire). And the first thing to check a match for the condition would be the techs owning empire.
Empires are not Objects, so that is simply unworkable

I am not sure if it would be feasible to add a dummy Empire object for scripting purposes. But having a random owned object as Source (in case you dont have a capital) strikes me as a waste of Source. So to rephrase my original idea:
  • add a dummy empire object so it can be Sourced and Targeted
  • tech gets activated for all the empires objects
  • the standard tech effects get gated by is-an-Empire condition
Probably the flexibility gained is not as high as effort and problems (and maybe runtime overhead) for such a change. Just trying to think of ways which would make FOCS more usable.

_________________
Any code or patches in anything posted here is released under the CC and GPL licences in use for the FO project.


Top
 Profile  
 
PostPosted: Thu Feb 22, 2018 10:07 pm 
Offline
AI Lead, Programmer
User avatar

Joined: Sat Sep 22, 2012 6:25 pm
Posts: 4648
Ophiuchus wrote:
So to rephrase my original idea:
  • add a dummy empire object so it can be Sourced and Targeted
  • tech gets activated for all the empires objects
  • the standard tech effects get gated by is-an-Empire condition
Probably the flexibility gained is not as high as effort and problems (and maybe runtime overhead) for such a change. Just trying to think of ways which would make FOCS more usable.
It's not apparent to me why you would think this makes FOCS any more usable, or how your "dummy" object would be easier to remember as Tech Source than the empire Capital (or the current fallbacks, which are essentially the same as your dummy object, it seems). You would also be adding in restrictions that an empire's techs can only apply to things owned by the empire, which would bar techs like Mines. I'm afraid I'm just not seeing any benefit at all.

_________________
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0


Top
 Profile  
 
PostPosted: Sat Feb 24, 2018 2:16 am 
Offline
Programmer

Joined: Sun Feb 14, 2016 12:08 am
Posts: 389
Quote:
I wonder if it should be made to do what it appears to, and not have the implicit contain feature for those types of conditions

Yes, adding a size/type/etc argument to Planet should only constrain the potential match.

@Dilvish I would have opened a bug report (since it is not an immediate priority) but curious if the scope should expand to encompass other conditions, is this limited to only Planet?


Top
 Profile  
 
PostPosted: Sat Feb 24, 2018 2:47 am 
Offline
AI Lead, Programmer
User avatar

Joined: Sat Sep 22, 2012 6:25 pm
Posts: 4648
dbenage-cx wrote:
@Dilvish I would have opened a bug report (since it is not an immediate priority) but curious if the scope should expand to encompass other conditions, is this limited to only Planet?
Open a bug report about making the script more efficient, or about changing the way these Conditions work? Neither really seems like a bug report to me.

The is-or-is-contained-by for these tests is documented as how they are supposed to work. If you think we should consider changing the operation then you could open up another thread here to focus on that topic, it is essentially a programming design debate and I think a forum thread is the appropriate place for that discussion if we are going to have it.

As for other conditions, Star Type is explicitly mentioned in the Effects Details page as another such case (and one that gets used fairly frequently I think). I can't think of any other similar kinds of relationships where it would even be a possibility. Well, except for the base Planet and Star conditions, conceivably they could also be expanded that way, but I think there would be a poor tradeoff of functionality to do it for them, it seems to me.

That fact that the the operation is not totally intuitive is certainly a reasonable factor to consider, but now that I am reminded about it there does seem to be good reason for those conditions to operate the way they do. It had just caught my eye while I was focused on trying to debug the regen issue, and then I got a bit interested in possibly making those particular scripts a bit more efficient.

_________________
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0


Top
 Profile  
 
PostPosted: Sun Feb 25, 2018 7:01 pm 
Offline
Psionic Snowflake

Joined: Tue Sep 30, 2014 10:01 am
Posts: 485
Dilvish wrote:
Ophiuchus wrote:
So to rephrase my original idea:
  • add a dummy empire object so it can be Sourced and Targeted
  • tech gets activated for all the empires objects
  • the standard tech effects get gated by is-an-Empire condition
Probably the flexibility gained is not as high as effort and problems (and maybe runtime overhead) for such a change. Just trying to think of ways which would make FOCS more usable.
It's not apparent to me why you would think this makes FOCS any more usable,

Because you can use ships or planets as Source objects for researched tech. And its easy to reference parts of the Source.

Dilvish wrote:
or how your "dummy" object would be easier to remember as Tech Source than the empire Capital (or the current fallbacks, which are essentially the same as your dummy object, it seems).
I agree that the Empire as a dummy doesnt add any value. I was thinking further that if you have an Empire object (or a set of those) you could interact more with it in FOCS; e.g. setting empire meters

Dilvish wrote:
You would also be adding in restrictions that an empire's techs can only apply to things owned by the empire, which would bar techs like Mines.

Im actually not restricting, Im extending. Actually mines would be more natural to script with my suggestion.

_________________
Any code or patches in anything posted here is released under the CC and GPL licences in use for the FO project.


Top
 Profile  
 
PostPosted: Sun Feb 25, 2018 7:42 pm 
Offline
AI Lead, Programmer
User avatar

Joined: Sat Sep 22, 2012 6:25 pm
Posts: 4648
Ophiuchus wrote:
Quote:
It's not apparent to me why you would think this makes FOCS any more usable,

Because you can use ships or planets as Source objects for researched tech. And its easy to reference parts of the Source.
We apply techs to planets and ships already quite easily, I don't see anything gained by this-- please give some kind of actual example of a use-cases for your different categories that you think would be better.

Ship Part effects already have their ship as Source, and in those cases where we want to work specifically with the parts we already have an easy time doing so, and those can (and many already do) easily take into account what techs an empire has. Again, a specific example might illuminate your point.

Quote:
Dilvish wrote:
You would also be adding in restrictions that an empire's techs can only apply to things owned by the empire, which would bar techs like Mines.

Im actually not restricting, Im extending. Actually mines would be more natural to script with my suggestion.
Your words were
Quote:
the standard tech effects get gated by is-an-Empire condition
We normally interpret "gate" as a restriction, I don't understand how you would mean for that to work as an extension, or why you think it would make mines more natural-- perhaps again be more specific.

_________________
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0


Top
 Profile  
 
PostPosted: Mon Feb 26, 2018 2:25 pm 
Offline
Psionic Snowflake

Joined: Tue Sep 30, 2014 10:01 am
Posts: 485
Dilvish wrote:
please give some kind of actual example of a use-cases for your different categories that you think would be better.

Im afraid at the moment I cant be very specific. This stuff usually comes up while I script something. I usually bang my head against it and find a different way. Sometimes this means I cant put the effect code where I think it should be from a maintenance point of view.

For example if i cant refer to the planet in the way i need it, i move the effect from the tech script to species scripts.

Quote:
Dilvish wrote:
We normally interpret "gate" as a restriction, I don't understand how you would mean for that to work as an extension, or why you think it would make mines more natural-- perhaps again be more specific.

You just picked a part of what I wrote. "tech gets activated for all the empires objects" is the extension. "the standard tech effects get gated by is-an-Empire condition" would be necessary for a 1-to-1 translation to get the current behavior back. This part is only technical. The important part is if it is worthwhile to have ships and buildings available as Source or not.

_________________
Any code or patches in anything posted here is released under the CC and GPL licences in use for the FO project.


Top
 Profile  
 
PostPosted: Mon Feb 26, 2018 7:29 pm 
Offline
AI Lead, Programmer
User avatar

Joined: Sat Sep 22, 2012 6:25 pm
Posts: 4648
Ophiuchus wrote:
You just picked a part of what I wrote. "tech gets activated for all the empires objects" is the extension. "the standard tech effects get gated by is-an-Empire condition" would be necessary for a 1-to-1 translation to get the current behavior back.
OK, I think I am understanding your idea better now-- you would have the Scope of a Tech automatically apply to all empire owned objects unless the scope further restricts it, is that right? But it would still somehow also be possible to have the scope include things that were not empire-owned-objects? Perhaps I am misunderstanding again, because I don't see how those two sentences could be reasonably compatible.

Quote:
Im afraid at the moment I cant be very specific. This stuff usually comes up while I script something. I usually bang my head against it and find a different way. Sometimes this means I cant put the effect code where I think it should be from a maintenance point of view. For example if i cant refer to the planet in the way i need it, i move the effect from the tech script to species scripts.... The important part is if it is worthwhile to have ships and buildings available as Source or not.
Well, absent an identifiable use-case, it seems pretty clear to me that it would not currently be worthwhile. We could simply put the conversation on hiatus until you again encounter (or just think of) a specific situation where you think it would be helpful.

_________________
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0


Top
 Profile  
 
PostPosted: Tue Feb 27, 2018 11:10 am 
Offline
Psionic Snowflake

Joined: Tue Sep 30, 2014 10:01 am
Posts: 485
Dilvish wrote:
Ophiuchus wrote:
You just picked a part of what I wrote. "tech gets activated for all the empires objects" is the extension. "the standard tech effects get gated by is-an-Empire condition" would be necessary for a 1-to-1 translation to get the current behavior back.
OK, I think I am understanding your idea better now-- you would have the Scope of a Tech automatically apply to all empire owned objects unless the scope further restricts it, is that right?

Almost yes :)
To clarify its not about the target scope but about the Source objects. So source would be all empire owned objects and you would use the is-an-Empire (or is-the-Empire-object) condition to restrict it to a single object in order to fire the effects only once.

Dilvish wrote:
But it would still somehow also be possible to have the scope include things that were not empire-owned-objects? Perhaps I am misunderstanding again, because I don't see how those two sentences could be reasonably compatible.

Well, yes and no. Yes, you can have anything you like in the target scope (exactly like now). No, I did not intend to include all objects as possible Source.

Dilvish wrote:
Well, absent an identifiable use-case, it seems pretty clear to me that it would not currently be worthwhile. We could simply put the conversation on hiatus until you again encounter (or just think of) a specific situation where you think it would be helpful.

Agreed

_________________
Any code or patches in anything posted here is released under the CC and GPL licences in use for the FO project.


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 13 posts ] 

All times are UTC


Who is online

Users browsing this forum: No registered users and 1 guest


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
cron
Powered by phpBB® Forum Software © phpBB Group