OpenTTD icon indicating copy to clipboard operation
OpenTTD copied to clipboard

Add: [Script] Unbunch order flag

Open SamuXarick opened this issue 1 year ago • 5 comments

Motivation / Problem

Missing unbunch feature for AIs.

Description

Allow AIs to unbunch vehicles. Proper handle the new OF_UNBUNCH_IN_DEPOT order flag when combined with other existing order flags. Add regression tests to test this flag and the returned error strings associated with it.

Limitations

Only for API version 15?

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR touches english.txt or translations? Check the guidelines
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, game_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

SamuXarick avatar Mar 09 '24 21:03 SamuXarick

Are there many script developers that are interested in checking for five extra error conditions in their code?

That there are many strings is just to provide the user some more user friendly explanation what's wrong. With scripts you should be more concise as returning too many errors is only making handling them correctly more cumbersome.

rubidium42 avatar Mar 10 '24 18:03 rubidium42

Would probably be better to keep error as it was, but maybe add way for scripts to query extra message if they need to.

glx22 avatar Mar 10 '24 18:03 glx22

Are there many script developers that are interested in checking for five extra error conditions in their code?

That there are many strings is just to provide the user some more user friendly explanation what's wrong. With scripts you should be more concise as returning too many errors is only making handling them correctly more cumbersome.

I can group the unbunch errors into 1 single message. But then it's going to take a bit more effort to identify the exact problem to solve it. Possible causes for the error would have to be properly documented.

SamuXarick avatar Mar 10 '24 18:03 SamuXarick

Would probably be better to keep error as it was, but maybe add way for scripts to query extra message if they need to.

Those errors in the form of two strings can't be reached from the GUI as far as I see. They're being handled in order_gui.cpp, CmdInsertOrder isn't called. Also, there's already inconsistency about the way the error is returned for a conditional order or full load order. Those are handled with return_cmd_error.

SamuXarick avatar Mar 10 '24 18:03 SamuXarick

But ScriptInstance::DoCommandCallback could access the extra message.

glx22 avatar Mar 10 '24 18:03 glx22