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

Multiple improvements to converter 3 to 4. #68724

Closed
wants to merge 1 commit into from

Conversation

FeralBytes
Copy link
Contributor

@FeralBytes FeralBytes commented Nov 16, 2022

Catch:
Thread.is_active() to is_alive().
Signal "idle_frame" to "process_frame"
Settings for Window size width/height to viewport_width/height. OS.real_window_size(), get_screen_size(), get_window_size() to DisplayServer equivalents.
OS.get_system_time_secs() to Time.get_time_dict_from_system()['seconds'].

I currently removed the fix for: String.plus_file() to path_join() because the Linux mono build says the function does not exist.
I removed get_network peer fix because it too does not match a direct function call since it is actually a method call which returns an object to call a second method in order to fix the Godot 3 sequence. get_network_peer() -> get_multiplayer().get_multiplayer_peer()

I also think it would be easy to fix:

  • Engine.editor_hint to Engine.is_editor_hint()
  • MainLoop.NOTIFICATION_WM_QUIT_REQUEST to NOTIFICATION_WM_CLOSE_REQUEST
  • MainLoop.NOTIFICATION_ to NOTIFICATION_

Although I think it would be easy to implement the above I also think I would break your code trying to do so.
Everything else that I mentioned here will be much more difficult.

@FeralBytes
Copy link
Contributor Author

FeralBytes commented Nov 16, 2022

Missing GDScript function in pair (get_network_peer - ===> get_multiplayer().get_multiplayer_peer <===)
This failure is valid, but not sure how to handle that change, any suggestions or just remove it for now?
Missing GDScript function in pair (plus_file - ===> path_join <===)
However is not valid. I am using path_join() right now in a beta4 project.

Of note I followed the directions that the Static Checks produces and attempted to do a git commit --amend. That does not work. So how would you like to proceed to fix my git history? Sorry I am not a git master. I know just enough to be dangerous.

If need be I can close this PR and create a fresh one with the changes that I have made, just let me know.

@Calinou
Copy link
Member

Calinou commented Nov 16, 2022

Of note I followed the directions that the Static Checks produces and attempted to do a git commit --amend. That does not work.

Since you have more than 1 commit in your branch, you need to perform a rebase instead of amending your latest commit.

Catch get_network peer.
Thread.is_active() to is_alive().
String.plus_file() to path_join()
Signal "idle_frame" to "process_frame"
Settings for Window size width/height to viewport_width/height.
OS.real_window_size(), get_screen_size(), get_window_size() to
DisplayServer equivalents.
OS.get_system_time_secs() to Time.get_time_dict_from_system()['seconds'].
@FeralBytes
Copy link
Contributor Author

Thanks @Calinou. I believe that should fix the git history.

@FeralBytes
Copy link
Contributor Author

This PR: #69712 will result in my PR needing a change too.

@akien-mga akien-mga requested a review from qarmin December 12, 2022 07:45
@Riteo
Copy link
Contributor

Riteo commented Dec 12, 2022

@FeralBytes it has already been merged BTW, you can already update your PR if that's what you're wondering.

@YuriSizov YuriSizov modified the milestones: 4.0, 4.1 Feb 10, 2023
@YuriSizov
Copy link
Contributor

Needs to be rebased, but also will have to wait for the time after 4.0 (can probably be merged in 4.0.x, definitely in 4.1, if approved).

@YuriSizov YuriSizov modified the milestones: 4.1, 4.2 Jun 14, 2023
@FeralBytes
Copy link
Contributor Author

It is stale now and has conflicts. Closing.

@FeralBytes FeralBytes closed this Aug 14, 2023
@AThousandShips AThousandShips removed this from the 4.2 milestone Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants