Skip to content

Move development project to Godot 4.3#197

Merged
manuq merged 4 commits intomainfrom
move-project-to-43
Aug 16, 2024
Merged

Move development project to Godot 4.3#197
manuq merged 4 commits intomainfrom
move-project-to-43

Conversation

@manuq
Copy link
Copy Markdown
Contributor

@manuq manuq commented Aug 15, 2024

@manuq manuq force-pushed the move-project-to-43 branch from 65d0240 to b950051 Compare August 15, 2024 16:46
@dbnicholson
Copy link
Copy Markdown
Member

I think this is reasonable, particularly since the flatpak has been updated to 4.3, and that's the primary way most of our developers use Godot.

Did you open our project in 4.3? When I've done this before, it's reimported assets. Can you update the tests to use 4.3? Actually, we should probably run those with a matrix of several older Godot versions since we can't expect users to be update Godot.

Godot 4.3 adds `disable_embedded_bitmaps=true` in font file imports.

https://phabricator.endlessm.com/T35494
@dylanmccall
Copy link
Copy Markdown
Contributor

dylanmccall commented Aug 15, 2024

I think this is reasonable, particularly since the flatpak has been updated to 4.3, and that's the primary way most of our developers use Godot.

Did you open our project in 4.3? When I've done this before, it's reimported assets. Can you update the tests to use 4.3? Actually, we should probably run those with a matrix of several older Godot versions since we can't expect users to be update Godot.

Yep, it updates a bunch of .ttf.import files in GUT, adding disable_embedded_bitmaps=true. That's from godotengine/godot@911fa38.

I pushed 68522f8 just now with the new import files.

I also checked if there's a newer version of GUT, and there is a newer version but I don't think it has anything relevant to 4.3.

@dylanmccall dylanmccall force-pushed the move-project-to-43 branch 2 times, most recently from 11657b5 to bf1d181 Compare August 16, 2024 00:38
Copy link
Copy Markdown
Contributor

@dylanmccall dylanmccall left a comment

Choose a reason for hiding this comment

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

And I also went and added a matrix of Godot versions for running tests. The setup-godot hook is oddly persnickety about the Godot version ending with a .0 even though it doesn't actually anywhere else, but all's working :) There is in fact a failing test, which I can confirm fails on my end as well, so, yay, testing! We should fix that.

==============================================
= Run Summary
==============================================

res://tests/test_instruction_tree.gd
- test_multiple_entry_script
    [Failed]:  ["extends Node2D
    
    
    func _ready():
    	print({text})
    
    "] expected to equal ["extends Node2D
    
    
    func _ready():
    	print({text})
    
    	print({text})
    
    "]:  
          at line -1

---- Totals ----
Scripts           2
Tests             13
  Passing         12
  Failing         1
Asserts           520
Time              0.176s


---- 1 failing tests ----

@manuq
Copy link
Copy Markdown
Contributor Author

manuq commented Aug 16, 2024

Yes, I see the test failing in my end as well, hmm.

Duplicating the ready_block as ready_block_2 works with Godot 4.2, but
fails with Godot 4.3. Looks like ready_block_2 does not have the
following print_block properly. So, generate ready_block_2 and its
print_block_2 from duplicated general_blocks: "ready" and "print"
directly to avoid the issue with Godot 4.3.

https://phabricator.endlessm.com/T35494
@starnight
Copy link
Copy Markdown
Contributor

I pushed the commit ("Generate real nodes in test_multiple_entry_script()") to fix the error #197 (review).

@starnight starnight requested a review from dylanmccall August 16, 2024 08:55
@manuq
Copy link
Copy Markdown
Contributor Author

manuq commented Aug 16, 2024

I pushed the commit ("Generate real nodes in test_multiple_entry_script()") to fix the error #197 (review).

Excellent, thanks!

@manuq
Copy link
Copy Markdown
Contributor Author

manuq commented Aug 16, 2024

I tried the different DuplicateFlags combinations and couldn't make this work. I wonder if we found a regression in 4.3 in the duplicate() behavior. In any case, this the code generation is going to change soon, so let's go with @starnight commit.

@manuq manuq merged commit 28c3f0d into main Aug 16, 2024
@manuq manuq deleted the move-project-to-43 branch August 16, 2024 12:18
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.

4 participants