paulloz / godot-ink

Ink integration for Godot Engine.
MIT License
582 stars 41 forks source link

InitializeRuntimeStory is never called when passing in Ink Json String #88

Open a2937 opened 6 months ago

a2937 commented 6 months ago

Describe the bug

I continuously get a null reference exception in C# when I try to create a new Ink Story from some code I loaded. As I have found out, the runtimeStory variable is never actually initialized for some reason. Thusly, Initialize InitializeRuntimeStory() must not be called when I set the story text.

To Reproduce

Pass in the path of a valid Ink file. In my case it was "res://ink/core/main.ink".

using Godot;
using GodotInk;
using Ink;
using Ink.Runtime;
using System;

public partial class CustomSetup : Node
{
private InkStory Story;

  private void LoadInkStory(string path)
  {

    var storyText = Godot.FileAccess.Open(path, Godot.FileAccess.ModeFlags.Read);

    Compiler compiler = new Compiler(storyText.GetAsText(), new Compiler.Options
    {
        sourceFilename = path,
        errorHandler = InkCompilerErrorHandler,
        fileHandler = new FileHandler(
            System.IO.Path.GetDirectoryName(path) ?? ProjectSettings.GlobalizePath(path)
        ),
    });
    String actualStoryContents = compiler.Compile().ToJson();
    Story = InkStory.Create(actualStoryContents);
    GD.Print(Story.CanContinue); 
  } 

    private class FileHandler : IFileHandler
    {
        private readonly string rootDir;

        public FileHandler(string rootDir)
        {
            this.rootDir = rootDir;
        }

        public string ResolveInkFilename(string includeName)
        {
            var resolvedFile = System.IO.Path.Combine(rootDir, includeName);
            return resolvedFile;
        }

        public string LoadInkFileContents(string fullFilename)
        {
            fullFilename = fullFilename.Replace(":\\", "://").Replace("\\", "/");

            var text = Godot.FileAccess.Open(fullFilename, Godot.FileAccess.ModeFlags.Read).GetAsText();
            return text;
        }
    }
}

Environment

Additional context

I'll expand on my example with a minimum viable project when prompted for one.

paulloz commented 6 months ago

Thank you for the report. This is a use-case I did not consider, since compiling .ink files at runtime seems quite niche. I'll expand the API to allow it.

a2937 commented 6 months ago

One of the key things you'll need to do is change

https://github.com/paulloz/godot-ink/blob/10a8bed67545468f8b1ab9b66b2aadb4e3043837/addons/GodotInk/Src/InkStory.cs#L46-L52

to the following

public static InkStory Create(string rawStory)
{
    var story = new InkStory
    {
        RawStory = rawStory
    };
    return story;
}

Here's how I compile it after loading the text from the file system.

var storyText = Godot.FileAccess.Open(path, Godot.FileAccess.ModeFlags.Read);

Compiler compiler = new Compiler(storyText.GetAsText(), new Compiler.Options
{
    sourceFilename = path,
    errorHandler = InkCompilerErrorHandler,
    fileHandler = new FileHandler(
        System.IO.Path.GetDirectoryName(path) ?? ProjectSettings.GlobalizePath(path)
    ),
});
String actualStoryContents = compiler.Compile().ToJson();
Story = InkStory.Create(actualStoryContents);

And here's my File Handler. While by no means perfect, it lets users pick a story to run from the res folder.

    private class FileHandler : IFileHandler
    {
        private readonly string rootDir;

        public FileHandler(string rootDir)
        {
            this.rootDir = rootDir;
        }

        public string ResolveInkFilename(string includeName)
        {
            var resolvedFile = System.IO.Path.Combine(rootDir, includeName);
            return resolvedFile;
        }

        public string LoadInkFileContents(string fullFilename)
        {
            fullFilename = fullFilename.Replace(":\\", "://").Replace("\\", "/");

            var text = Godot.FileAccess.Open(fullFilename, Godot.FileAccess.ModeFlags.Read).GetAsText();
            return text;
        }
    }
paulloz commented 6 months ago

No, this is not how to do this, it is going to go through another dedicated creation method. We do not want to change the behaviour of the factory used by the importer.