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

[Bug]: ScriptList::FillList does not charge scripts any opcodes #12128

Open
JGRennison opened this issue Feb 19, 2024 · 3 comments
Open

[Bug]: ScriptList::FillList does not charge scripts any opcodes #12128

JGRennison opened this issue Feb 19, 2024 · 3 comments
Labels
component: AI/Game script (squirrel) This issue is related to Squirrel (Scripting language)

Comments

@JGRennison
Copy link
Contributor

Version of OpenTTD

master

Expected result

ScriptList::FillList should charge some reasonable number of opcodes, e.g. based on the size of the pool or the number of pool items which end up in the list.

Actual result

ScriptList::FillList doesn't charge anything, even though it may consume very large amounts of CPU-time. Charging a correspondingly large number of opcodes can mitigate the performance degradation caused by scripts.

Charging a large number of opcodes may also encourage script authors to pursue more efficient methods.

Steps to reproduce

See ScriptList::FillList

@glx22
Copy link
Contributor

glx22 commented Feb 19, 2024

Indeed filtering using API function is free, but using custom filter written in squirrel is not.
List constructors were free before introduction of FillList.

I agree applying filter while construction should have a cost, but I think it should be less than the Valuate cost (which is 5+call per item).

@JGRennison
Copy link
Contributor Author

The script should be charged even if a filter isn't used.

To take a random example, AIVehicleList() iterates the whole vehicle pool and populates it with all the companies head vehicles.
ScriptList is rather slow and memory-hungry so this wastes a lot of CPU time and memory even if nothing is filtered.
It would be a perverse incentive to make unfiltered lists free but charge for filtered lists, when making ScriptList instances as small as possible (e.g. by filtering) is preferable.

@PeterN
Copy link
Member

PeterN commented Feb 19, 2024

So basically charge by the number of results rather than the work required to produce the results.

JGRennison added a commit to JGRennison/OpenTTD-patches that referenced this issue Feb 20, 2024
@2TallTyler 2TallTyler added the component: AI/Game script (squirrel) This issue is related to Squirrel (Scripting language) label Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: AI/Game script (squirrel) This issue is related to Squirrel (Scripting language)
Projects
None yet
Development

No branches or pull requests

4 participants