Add fromInt method on doc_gen - #2091
Conversation
| return 0; | ||
| } | ||
|
|
||
| public static function fromSources(gl:Dynamic, source:String, type:Int):GLShader |
There was a problem hiding this comment.
While we're here, I think fromSource should be fromSources so the doc signature matches the real one. It seems it was a typo from c9d522d. GLShader.fromSource takes one source, unlike GLProgram.fromSources which takes two sources.
There was a problem hiding this comment.
Change the signature, now CI is failing with
Error: Incompatibilities between lime.graphics.opengl.GLShader in Android and HTML5 (could not merge definition)
should I change it back to dynamic or something?
There was a problem hiding this comment.
should I change it back to dynamic or something?
Yeah, that could be the reason
There was a problem hiding this comment.
Hm, I think there was probably a reason it is Dynamic anyway, so it should probably be kept like that. However, it seems that's not why it was failing, and on second thought it's probably adding #if !lime_webgl that introduced this error. Maybe we actually want to mark fromInt as noDoc or something.
There was a problem hiding this comment.
Can you be more specific? I'm not familiar with noDoc, I assume it's a meta? would I add it to the new method, or both
There was a problem hiding this comment.
Could also wrap this in #if haxe > version("4.3"), I suppose. I don't get why you would generate docs in old haxe versions, openfl.org doesn't seem to show apis for older versions
There was a problem hiding this comment.
I think adding that check around fromInt would just bring back the original error: has no field fromInt
To get the ci passing, we can change the haxe version here:
https://github.com/Geokureli/lime/blob/f8ba43823ed773c04f88b12603c79edbd739cefa/.github/workflows/main.yml#L733
Maybe we can just update the global version to haxe 4.3, unless there was a reason lime is sticking to 4.2 for now. Perhaps @joshtynjala might know?
There was a problem hiding this comment.
I think adding that check around fromInt would just bring back the original error:
has no field fromInt
It would fix my issue with flixel-docs and anyone else using haxe 4.3. but yeah, let's find out
There was a problem hiding this comment.
I don't recall any particular reason for us to strictly stick to Haxe 4.2 instead of 4.3 for our default on CI. I think it's been working fine, so we haven't bothered updating. 4.3 has been out for a while now, and is also very stable, so it shouldn't cause issues.
There was a problem hiding this comment.
In that case I think we can change the global haxe version to 4.3.7 in this PR, which should make the docs ci job pass here.
3 participants
Flixel-doc was getting the following error on this line:
I was gonna make an issue and fix later but I saw your new 12 part questionnaire to make a bug and decided I'll probably never ever want to go through that, especially for something like this that takes 2 sentences to properly convey.