Skip to content

Revert TTS#1889

Merged
vadi2 merged 2 commits intoMudlet:developmentfrom
vadi2:revert-tts
Aug 13, 2018
Merged

Revert TTS#1889
vadi2 merged 2 commits intoMudlet:developmentfrom
vadi2:revert-tts

Conversation

@vadi2
Copy link
Copy Markdown
Member

@vadi2 vadi2 commented Aug 13, 2018

Brief overview of PR changes/additions

Revert recent TTS additions until we can fix the discovered issues.

Motivation for adding to Mudlet

There are a lot of issues with it, see #1881.

Other info (issues closed, discussion etc)

We'll concentrate on fixing the issues and getting the features back in for the next release.

@vadi2 vadi2 requested a review from a team as a code owner August 13, 2018 08:42
@vadi2 vadi2 requested review from a team August 13, 2018 08:42
@vadi2
Copy link
Copy Markdown
Member Author

vadi2 commented Aug 13, 2018

@Kebap here it is reverted - have not tested it myself yet, can't right now.

Copy link
Copy Markdown
Member

@SlySven SlySven left a comment

Choose a reason for hiding this comment

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

I not sure everything should be reverted here...

apt:
sources: &add-sources
- sourceline: 'ppa:beineri/opt-qt571-trusty'
- sourceline: 'ppa:beineri/opt-qt-5.10.1-trusty'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Apart from the proposed removed of qt510speech do we need/want to revert to 5.9 - or was 5.10 the possible cause of whatever problems there was/is with TTS? This comment applies to all the currently proposed changes in this file.

set(FONT_TEST $ENV{WITH_FONTS})
if(DEFINED FONT_TEST)
string(TOUPPER ${FONT_TEST} FONT_TEST)
string(TOUPPER ${FONT_TEST} FONT_TEST)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ℹ️ I think the indentation for CMake files is two spaces...

file(GLOB_RECURSE lua_files RELATIVE "${CMAKE_HOME_DIRECTORY}/src/mudlet-lua/lua/" "${CMAKE_HOME_DIRECTORY}/src/mudlet-lua/lua/*.lua")
list(LENGTH lua_files lua_file_count)
if(lua_file_count EQUAL 26)
if(lua_file_count EQUAL 25)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry if changing this each time the file count changes is a PITA but I put this warning in to try and prevent CMake from silently getting out of date from changes made in the qmake build system.

# This is not present before Qt 5.7:
find_package(Qt5 COMPONENTS Gamepad QUIET)

# This is not present before Qt 5.9:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I thought we are still building against Qt 5.7 so handling Qt modules that may not (OS permitting) be available means this should be left in IMHO as should the comments following about which versions should have what - it may help when debugging why something doesn't work on a particular OS/Qt version as it all helps to document our expectations.

// Note: Optional forceGlobal allows passing a null pointer as pHost, and will raise
// an event for all profiles.
void HostManager::postInterHostEvent(const Host* pHost, const TEvent& event, const bool forceGlobal)
void HostManager::postInterHostEvent(const Host* pHost, const TEvent& event)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The changes here and in src/HostManager.cpp do not seem to need/want reversion?

macx:QMAKE_MACOSX_DEPLOYMENT_TARGET = 10.11

QT += network opengl uitools multimedia gui concurrent
qtHaveModule(gamepad) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Like in the CMake project file case - we still cover Qt 5.7 and we need to cover that older version without some modules don't we?

@vadi2
Copy link
Copy Markdown
Member Author

vadi2 commented Aug 13, 2018

@SlySven I just want a simple revert that we can then unrevert and fix - I don't want to mess it up with commits.

Copy link
Copy Markdown
Member

@SlySven SlySven left a comment

Choose a reason for hiding this comment

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

Okay, but some of those Qt things will break things and need fixing - I think...

@vadi2 vadi2 merged commit 8b6641b into Mudlet:development Aug 13, 2018
@vadi2 vadi2 deleted the revert-tts branch August 13, 2018 15:15
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.

2 participants