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

Type Dynamic callbacks in Assets #1477

Merged
merged 2 commits into from
Nov 9, 2023
Merged

Type Dynamic callbacks in Assets #1477

merged 2 commits into from
Nov 9, 2023

Conversation

RblSb
Copy link
Contributor

@RblSb RblSb commented Nov 4, 2023

I spended enough hours today debugging why this (not my) silly code breaks on cpp:

Assets.loadEverything(function () {
	// Avoid passing update/render directly,
	// so replacing them via code injection works
	Scheduler.addTimeTask(function () { update(); }, 0, 1 / 60);
	System.notifyOnFrames(function (frames) { render(frames); });
}, (err:AssetError) -> {
	throw err;
});

I want to improve this api, which will also fix future dynamic problems, like dynamicObj.name.contains failure on some static targets.
With typed callbacks this should be harder to break and easier to use now.
Also, is this possible to get shaders in assets loader? Seems like not, or there is some ways to enable runtime loading?

@RobDangerous
Copy link
Member

I'm sorry but... I don't know what you are talking about.

@RblSb
Copy link
Contributor Author

RblSb commented Nov 5, 2023

This PR solves two problems:

Assets.loadEverything(() -> {}, (err:AssetError) -> {
	throw err;
});

Will be detected as (callback, null, null, failed), instead of (callback, filter), since callbacks are not Dynamic anymore.
And it also solves problem, when you try to access item.name fields, like:

Assets.loadEverything(() -> {}, item -> {
return item.name.contains("menu");
});

Since item.name will be typed as String, and not Dynamic with unknown native contains method (for example, js has str.includes, but no str.contains)

@RobDangerous
Copy link
Member

OK. But I originally made those dynamic because I wanted to add the option to add any data to assets in the khafile. Can typedefs support that?

@RblSb
Copy link
Contributor Author

RblSb commented Nov 7, 2023

There is no typedef A = {foo:Int} & Dynamic; support in haxe yet, but if you think about making this feature before that support, i can change this typedef to abstract, that will provide completion for item.name / other fields with good types, but also allow item.anythingElse fields and return Dynamic values from them.

@RobDangerous
Copy link
Member

Yes, let's make some abstract art.

@RobDangerous RobDangerous merged commit 6cd94c8 into Kode:main Nov 9, 2023
23 checks passed
@RblSb RblSb deleted the safe_assets branch November 9, 2023 12:59
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