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

TTS_Linux: Fix size_t template issue on OpenBSD by using int consistently #84017

Merged
merged 1 commit into from
Oct 27, 2023

Conversation

rfht
Copy link
Contributor

@rfht rfht commented Oct 26, 2023

This is a follow-up of #83804. The issue was observed on OpenBSD where compilation failed because of a missing GetTypeInfo<unsigned long> template. Using uint64_t for TTS_Linux::_speech_index_mark() and TTS_Linux::_speech_event() arguments fixes this.

Bugsquad edit:

@omar-polo
Copy link
Contributor

With this (plus some other "usual" patches like the one for the sndio backed) I'm finally able to compile and run godot on OpenBSD!

I wasn't sure about the cast, but we hit already a similar issue in the embedded copy of embree (https://github.com/openbsd/ports/blob/master/games/godot/patches/patch-thirdparty_embree_common_math_math_h). I guess that having size_t a typedef for unsigned long is not that common?

To be fair I'm still not completely sure why it was failing to build before, I got the cast almost by chance.

Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

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

p_client_id is not used anywhere, so there's no need to pass it to deferred calls. And message ID is limited to int range (in case events it should be always > 0, -1 is used for errors).

platform/linuxbsd/tts_linux.cpp Outdated Show resolved Hide resolved
platform/linuxbsd/tts_linux.cpp Outdated Show resolved Hide resolved
platform/linuxbsd/tts_linux.cpp Outdated Show resolved Hide resolved
platform/linuxbsd/tts_linux.cpp Outdated Show resolved Hide resolved
platform/linuxbsd/tts_linux.h Outdated Show resolved Hide resolved
@fire fire changed the title fix issue with size_t template issue by using uint64_t consistently Fix issue with size_t template issue by using uint64_t consistently Oct 26, 2023
rfht added a commit to jasperla/openbsd-wip that referenced this pull request Oct 26, 2023
Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

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

Please squash the commits, see Pull request workflow for details.

@akien-mga akien-mga added this to the 4.2 milestone Oct 27, 2023
@akien-mga akien-mga added the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Oct 27, 2023
@akien-mga akien-mga changed the title Fix issue with size_t template issue by using uint64_t consistently TTS_Linux: Fix size_t template issue on OpenBSD by using int consistently Oct 27, 2023
@akien-mga
Copy link
Member

Sorry I should have mentioned this earlier: the commit message is incorrect since it's not using uint64_t anymore.
Could you amend it to use something like what I used for the PR title? This makes both the fix and the context more explicit.

@akien-mga akien-mga merged commit b1ae184 into godotengine:master Oct 27, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

@rfht
Copy link
Contributor Author

rfht commented Oct 27, 2023

actually the third, but still an opportunity to celebrate!
1fe6ad6
204de5e

@YuriSizov YuriSizov removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Jan 23, 2024
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.1.4.

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.

OpenBSD error: conversion from 'const Variant' to 'unsigned long' is ambiguous
5 participants