P1X-in / tanks-of-freedom-ii

Indie Turn Based Strategy in Isometric Voxel Art http://tof.p1x.in
Other
221 stars 20 forks source link

Static typing #23

Open dfgworm opened 5 months ago

dfgworm commented 5 months ago

I am not entirely sure what you mean by this milestone, there are multiple ways to achieve it. First is const x := preload(...) which i personally have had problems with. Even in smaller projects things that use preload just randomly break sometimes, not loading scenes or scripts and showing bullshit errors. Not even sure if 4.3 solves it entirely.

Second option is using class_name X on top of scripts. I use this option extensively, basically just naming every single class and using them as types similar to int or float. It works perfectly:

class_name WallBreaker
extends Node
@onready var wallBreaker:WallBreaker = $WallBreaker
@onready var oreBreaker:OreBreaker = $OreBreaker
var health := entity.get_node_or_null("Health") as HealthComponent

Class name gets registered globally, which is often very convenient, and also allows the usage of static vars and funcs.

Of course, godot still has no namespaces for this feature, which can create a problem of conflicts and pollution of global space, but i do not think your project's scale is large enough to worry about that.

I'd suggest transitioning to the second option. Having everything (almost, i don't do that for most of UI) as a globally named class makes coding experience very similar to a fully typed language, like C#.

czlowiekimadlo commented 5 months ago

I'm well aware of how to do this :) The issues I have encountered are that I hit cyclic dependencies very quickly.

dfgworm commented 5 months ago

But cyclic dependencies are not a problem with global classes? At least i have never encountered anything like that. Cyclic references is probably what breaks preload.

dfgworm commented 5 months ago

Yes, i am pretty sure everything is fine with the usage of class_name

I have this code in a project that i have been working on for months without any problems:

class_name World
extends TileMap

static var instance:World

static var roomManager :RoomManager:
    get: return instance.get_node(^"RoomManager")
class_name RoomManager
extends Node2D

@onready
var world :World = get_parent()

I am pretty sure i have a shitton of other cyclic references everywhere. The only time i had weird error messages and corrupted scenes was right when i used preload, which i stopped doing. Never had any trouble since then.

czlowiekimadlo commented 5 months ago

But cyclic dependencies are not a problem with global classes? At least i have never encountered anything like that. Cyclic references is probably what breaks preload.

It very well may be an issue with preload, but the fact is I tried using class_name and got cyclic dependencies very quickly. I do know that what you have presented in your example works, I have tested this myself. Yet in the project it breaks. Considering that I could drop all preloads from the codebase, I might have to put in a bit more effort for this to work.

dfgworm commented 5 months ago

As i said, i have used class_name really extensively throughout all my projects since Godot 4.0 came out, and have never encountered any problems. Perhaps i did not have a large enough project, but i ALWAYS static type any variable that uses classes. I am pretty sure i would have encountered problems if there were any. I also browsed issues related to this, and this seems to be marked as fixed: https://github.com/godotengine/godot/issues/21461

I have made a pr to your static-typing branch, introducing some global classes: https://github.com/P1X-in/tanks-of-freedom-ii/pull/24 And it seems to be working fine.

I have disabled typing warnings for now, since they create a ton of messages. Frankly, i would not add types for every number and string variable in the repo, those really do not add any value. IMO the most important vars are the ones that contain classes, which allow you to see that juicy autocomplete for functions and variables. I have no idea how you even managed to get to such a scale without func autocompletion. I think i would have gone insane from remembering all those names.

I can continue changing things in this direction. though i am not sure if i should work on that branch, as it is quite a bit behind on commits, and changes such as mine are ought to cause merge conflicts. Maybe i should redo those changes on the main branch? I have used exclusively ctrl+shift+R for these changes, so it didn't really take much time. Or maybe merge conflicts are not too bad, idk.

Also, maybe you can tip me what exactly to change? Where did you encounter problems before? It would be good to work on problematic parts first.

czlowiekimadlo commented 4 months ago

As i said, i have used class_name really extensively throughout all my projects since Godot 4.0 came out, and have never encountered any problems.

And I have encountered it very quickly 🤷🏻

I have made a pr to your static-typing branch, introducing some global classes:

24

And it seems to be working fine.

As you have already noticed, this branch is outdated. There isn't much sense in doing anything with it in it's current state.

I have disabled typing warnings for now, since they create a ton of messages.

The intention is on the contrary - to have them bumped up to errors.

Frankly, i would not add types for every number and string variable in the repo, those really do not add any value. IMO the most important vars are the ones that contain classes, which allow you to see that juicy autocomplete for functions and variables.

My intention is to have everything fully typed.

I have no idea how you even managed to get to such a scale without func autocompletion. I think i would have gone insane from remembering all those names.

I avoid using intellisense-style autocompletion because I have seen enough developers who are completely clueless once it breaks. And it always breaks at some point. I use token-based autocompletion instead. And even that breaks sometimes, but I don't care too much because I can handle it.

Also, maybe you can tip me what exactly to change? Where did you encounter problems before? It would be good to work on problematic parts first.

I'm afraid I don't have much in terms of tips. The current PR is already too much for me to follow or review. All of this is just a hobby project, it doesn't really have any goals.

dfgworm commented 4 months ago

And it always breaks at some point

It is actually pretty robust.

var unit := _typeless as BaseUnit

You can always declare a new variable with the type you are using, even if you are in a completely typeless environment. The only time you can't use it is when you are dealing with objects that don't have a common base class. As for it breaking in massive code bases, that is annoying, but it usually still works at least half of the time.

In the end, it's really nice to have features of autocomplete and ctrl-clicking to hop around, even if they don't work sometimes. Just because you can't always use a car doesn't mean you have to exclusively ride a bike.

Anyway, should i even commit such massive changes with typing? They are pretty all-encompasing, and i do not have the full grasp of your code. I understand having someone else do that in your personal project might not be desirable.