Skip to content

URL processing options, useful when saving scene to file#636

Merged
michaliskambi merged 57 commits into
masterfrom
url-processing
Apr 21, 2026
Merged

URL processing options, useful when saving scene to file#636
michaliskambi merged 57 commits into
masterfrom
url-processing

Conversation

@janadamec1

Copy link
Copy Markdown
Member

This adds ProcessUrls procedure with several options in TUrlProcessing:

  • suChangeCastleDataToRelative: changes castle-data: URLs to relative paths to target scene filename
  • suChangeAllPathsToRelative: changes all paths to relative paths
  • suEmbedResources: stores all resources directly inside the scene
  • suCopyResourcesToSubdirectory: copies resources to "-resources" subdirectory and stores the reference as relative URL

Usage is implemented in CastleEditor, deprecated_library, and url-processing branch in CastleModelViewer/Converter. The check in GUI, the options are exposed as Radio menu items under Save As... command.

@michaliskambi

Copy link
Copy Markdown
Member

Thank you!

Please add some automatic tests (1. that ProcessUrls works right, 2. that DeepCopy of prototypes works right). Because this is a difficult thing to test, there are various options. Let's use automated testing to help us (and ensure it will keep working forever :) ).

Short primer how to add tests:

You can test running the test suite in "tests/" , see tests/README.md . Short version: compile the project there, and run it :) At the beginning, just run all tests, to make sure it is OK (it should be -- GitHub Actions also check it).

I usually use the "--console" and "--filter xxx" options when developing a new test. So I would add a method like "TestProcessUrls" to TTestX3DNodes (or maybe several methods) and then continue checking

cd tests/
castle-engine compile --mode=debug
./castle-tester --console --filter=TTestX3DNodes.TestProcessUrls

Of course you can compile and run the testsuite as you like, and use GUI or console runner, whatever is comfortable to you.

The tests don't need to be anything complex, it can be as simple as providing some hardcoded input and looking does it match hardcoded output. Like

procedure TTestX3DNodes.TestProcessUrlsEmbed;
begin
  RootNode := TX3DRootNode.Create;
  ...
  ImageTexture.SetUrl('textures/my.png');
  // add ImageTexture to some ShapeNode within RootNode

  TempLocation := GetTempFileName + '.x3d.';
  ProcessUrls(RootNode, TempLocation, suEmbedResources);

  AssertTrue(ImageTexture.Urls.Count = 1);
  AssertTrue(IsSuffix('data:', ImageTexture.Urls[0]));

  finally ... FreeAndNil everyhing :) end;
end;

@michaliskambi

Copy link
Copy Markdown
Member

Sorry for a delay with this, I have this on a list of TODOs for next week!

I see there are 2 conflicts with this PR, if you want to help me you can solve them. But you can also ignore them, I can do them too next week, look trivial :) Just conflicts around LPK / LPI that we indeed changed in CGE master in the meantime.

@janadamec1

Copy link
Copy Markdown
Member Author

No problem, I will merge it into master once the library_update PR is merged (working on it now)

@janadamec1 janadamec1 requested a review from michaliskambi March 7, 2026 16:57
@michaliskambi

Copy link
Copy Markdown
Member

I see you're updating this PR, thank you, and thanks a lot for the patience -- I know I have unreasonable delay now with reviewing.

I had busy February (2026), and March (2026) looks busy too, due to some non-computer stuff happening in my life, and also finalizing one job. I have added to TODO, again, to "review this PR" ASAP. I hope to get to it soon. I should have lots more time for CGE work since April 2026, so please bear with me until then :)

P.S. If GHA fails, note that we have now known issue, introduced by me, which makes GHA fail on "master" too. The PasDoc version in our Docker image is now too old. It should be good again in a few hours. (And in positive news, I hope to make PasDoc 1.0.0 release soon :) ).

@janadamec1

Copy link
Copy Markdown
Member Author

Thanks, merging this PR will make my life much easier.

janadamec1 and others added 19 commits March 17, 2026 11:41
…omeone solved conflict by copying whole file :) ), bring back ECgeGameControllerButton and ECgeKey mods from master
… EInternalError when eUrlProcessing is unexpected (otherwise UrlProcessing has undefined value)
…ssary - TX3DInterfaceDeclaration inside proto have ParentNode = nil, that's ok

Simplify code and add comments to make it more explicit.
- "<name>_resources" document,

- Download never returns nil,

- make try..finally correct (only free F once it is initialized),

- ResourceFileTitle -> ResourceFileBaseName (we never call this "title" in CGE),

- use InclPathDelim (just shorter name).

@michaliskambi michaliskambi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you again and sorry (for the last time in this PR! :) ) that this took me ages to review.

I've done some improvements, notably:

  • extended tests, to auto-test DeepCopy of prototypes
  • simplified DeepCopy of prototypes -- they don't need NewParentNode passed around
  • fixed 2 try..finally clauses, to only do finally if the freed pointer was initialized

... and I'm happy and merging this now:)

@michaliskambi michaliskambi merged commit d2fcf4e into master Apr 21, 2026
5 of 63 checks passed
@michaliskambi michaliskambi deleted the url-processing branch April 21, 2026 01:03
@michaliskambi

Copy link
Copy Markdown
Member

P.S. I'm aware that logical next step is to refresh branch url-processing of our model viewer (merging master into it, so that it's rebased on master) and applying it too. Added to my TODO. I you will make a PR with it, I will be grateful :)

@janadamec1

Copy link
Copy Markdown
Member Author

Thanks for merging and sorry for that wrong merge of library header.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants