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

Add: [Script] Basic information about loaded NewGRFs for scripts. #9464

Merged
merged 1 commit into from Aug 9, 2021

Conversation

@michicc
Copy link
Member

@michicc michicc commented Aug 9, 2021

Motivation / Problem

Currently, scripts use various heuristics to detect loaded NewGRFs that are inherently unreliable and prone to break.

At the same time, the list of loaded NewGRFs is easily accessible to a human player. Thus, giving scripts the same basic information is consistent with the approach to not give scripts any information advantage over a human player.

Description

This PR allows AIs and GS to get the most basic information about loaded NewGRFs. Static NewGRFs are hidden from scripts.
Right now there are no APIs that would allow a script to get more insight into what a NewGRF actually provides and changes (which matches the human experience).

Whether any other information should be exposed to scripts is up for discussion.

Limitations

The implementation is quite inefficient as the active NewGRF config is only kept in a non-indexable linked list.
Also, there is no way for a script to be informed about changes to loaded NewGRFs after game start.

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 affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')
Currently, scripts use various heuristics to detect loaded NewGRFs that are inherently unreliable.
The list of loaded NewGRFs is easily accessible to a human player, and thus giving
scripts the same information is consistent with the current approach to not give scripts
more information than a human player.
Copy link
Member

@TrueBrain TrueBrain left a comment

YOLO! What is the worst that can happen ;)

Loading

@michicc michicc merged commit 8706dcd into OpenTTD:master Aug 9, 2021
15 checks passed
Loading
@michicc michicc deleted the pr/script_newgrf branch Aug 9, 2021
@andythenorth
Copy link
Contributor

@andythenorth andythenorth commented Aug 10, 2021

UPDATE: issues in this comment resolved by #9465

Crashed it. Not sure if this is PEBKAC or not.

    foreach (grf, _ in GSNewGRFList()) {
        //Log.Info(grf, Log.LVL_INFO);
    }

image

I tried some crude debugging:

ScriptNewGRFList::ScriptNewGRFList()
{
	for (auto c = _grfconfig; c != nullptr; c = c->next) {
		printf("FOO %x\n", c->ident.grfid);
		if (!HasBit(c->flags, GCF_STATIC)) {
			//this->AddItem(c->ident.grfid);
			this->AddItem(0);
		}
	}
}

Which yields

FOO 10bfbfb
FOO 33535355
FOO feea8797
FOO 10e4e45
FOO 54504843
FOO 800a4444
FOO 504143
FOO 1035454
FOO 1f124143
FOO 80025f1
^[[AFOO 10bfbfb
FOO 33535355
FOO feea8797
FOO 10e4e45
FOO 54504843
FOO 800a4444
FOO 504143
FOO 1035454
FOO 1f124143
FOO 80025f1

--

Further debugging, this GS crash appears to be caused by Road Hog (both released and unreleased versions).

ScriptNewGRFList() works as expected if Road Hog is not in the grf list.

There may be other grfs that cause a crash.

image

--

I have a hunch that it's related to the Road Hog grfid, as Canadian BK Tunnels grf has a similar grfid starting with 9787 and also crashes.
I changed FIRS grfid to see what happened.

If FIRS-with-9787EAFE is in newgrf list, the GS crashes. I tried changing the position, seems to make no difference.

image

Loading

@andythenorth
Copy link
Contributor

@andythenorth andythenorth commented Aug 10, 2021

Also tested the loaded / version / name methods, works fine.

    if (GSNewGRF.IsLoaded(0x80025f1)) {
        Log.Info(GSNewGRF.IsLoaded(0x80025f1), Log.LVL_INFO);
        Log.Info(GSNewGRF.GetVersion(0x80025f1), Log.LVL_INFO);
        Log.Info(GSNewGRF.GetName(0x80025f1), Log.LVL_INFO);
    }

image

The endian-ness of the grfid was confusing, FIRS grfid is reported in OpenTTD newgrf window as 'F1250008'. Seems to have dropped a 0 also somewhere.

Unrelated: L48 of script_newgrf.hpp contains a bridge-related comment :) * Get the BridgeID of a bridge at a given tile.

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants