From 76be2e91e5f69b61077c8287331319097529492c Mon Sep 17 00:00:00 2001 From: "florent.teppe" Date: Sat, 18 Sep 2021 18:25:04 +0200 Subject: [PATCH 1/7] Fixed an issue where keyword search expected the text to be all ASCII characters --- apps/openmw/mwdialogue/keywordsearch.hpp | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/apps/openmw/mwdialogue/keywordsearch.hpp b/apps/openmw/mwdialogue/keywordsearch.hpp index f296f223fb..3cd8517651 100644 --- a/apps/openmw/mwdialogue/keywordsearch.hpp +++ b/apps/openmw/mwdialogue/keywordsearch.hpp @@ -68,6 +68,19 @@ public: return false; } + static bool isWhitespaceUTF8(const int utf8Char) + { + if (utf8Char >= 0 && utf8Char <= UCHAR_MAX) + { + //That function has undefined behavior if the character doesn't fit in unsigned char + return std::isspace(utf8Char); + } + else + { + return false; + } + } + static bool sortMatches(const Match& left, const Match& right) { return left.mBeg < right.mBeg; @@ -83,7 +96,7 @@ public: { Point prev = i; --prev; - if(isalpha(*prev)) + if(!isWhitespaceUTF8(*prev)) continue; } From bcb0526268ad529fe51929f4b21af12357b823a7 Mon Sep 17 00:00:00 2001 From: "florent.teppe" Date: Sun, 19 Sep 2021 15:00:51 +0200 Subject: [PATCH 2/7] Added climits, it compiled on VS2019, but not with g++ --- apps/openmw/mwdialogue/keywordsearch.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/openmw/mwdialogue/keywordsearch.hpp b/apps/openmw/mwdialogue/keywordsearch.hpp index 3cd8517651..fc6f2a91e8 100644 --- a/apps/openmw/mwdialogue/keywordsearch.hpp +++ b/apps/openmw/mwdialogue/keywordsearch.hpp @@ -2,6 +2,7 @@ #define GAME_MWDIALOGUE_KEYWORDSEARCH_H #include +#include #include #include #include From c1c501ca3559571c8292cd0f684274e09f07f9ff Mon Sep 17 00:00:00 2001 From: "florent.teppe" Date: Sun, 19 Sep 2021 15:05:48 +0200 Subject: [PATCH 3/7] Added test to make sure keyword search works even with non ascii characters --- .../mwdialogue/test_keywordsearch.cpp | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/apps/openmw_test_suite/mwdialogue/test_keywordsearch.cpp b/apps/openmw_test_suite/mwdialogue/test_keywordsearch.cpp index 431725be2c..25ea566165 100644 --- a/apps/openmw_test_suite/mwdialogue/test_keywordsearch.cpp +++ b/apps/openmw_test_suite/mwdialogue/test_keywordsearch.cpp @@ -65,3 +65,26 @@ TEST_F(KeywordSearchTest, keyword_test_conflict_resolution3) ASSERT_TRUE (matches.size() == 1); ASSERT_TRUE (std::string(matches.front().mBeg, matches.front().mEnd) == "bar lock"); } + + +TEST_F(KeywordSearchTest, keyword_test_utf8_word_begin) +{ + // We make sure that the search works well even if the character is not ASCII + MWDialogue::KeywordSearch search; + search.seed("états", 0); + search.seed("ïrradiés", 0); + search.seed("ça nous déçois", 0); + + + std::string text = "les nations unis ont réunis le monde entier, états units inclus pour parler du problème des gens ïrradiés et ça nous déçois"; + + std::vector::Match> matches; + search.highlightKeywords(text.begin(), text.end(), matches); + + ASSERT_TRUE (matches.size() == 3); + ASSERT_TRUE(std::string( matches[0].mBeg, matches[0].mEnd) == "états"); + ASSERT_TRUE(std::string( matches[1].mBeg, matches[1].mEnd) == "ïrradiés"); + ASSERT_TRUE(std::string( matches[2].mBeg, matches[2].mEnd) == "ça nous déçois"); + + //ASSERT_TRUE (std::string(matches.front().mBeg, matches.front().mEnd) == "bar lock"); +} From 0e06e9b22188e7f23feacd0ddef1cdc286a3f732 Mon Sep 17 00:00:00 2001 From: "florent.teppe" Date: Sun, 19 Sep 2021 20:29:32 +0200 Subject: [PATCH 4/7] Removed useless comment and converted the file to UTF8 to keep special characters --- .../mwdialogue/test_keywordsearch.cpp | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/apps/openmw_test_suite/mwdialogue/test_keywordsearch.cpp b/apps/openmw_test_suite/mwdialogue/test_keywordsearch.cpp index 25ea566165..d633feecd9 100644 --- a/apps/openmw_test_suite/mwdialogue/test_keywordsearch.cpp +++ b/apps/openmw_test_suite/mwdialogue/test_keywordsearch.cpp @@ -71,20 +71,18 @@ TEST_F(KeywordSearchTest, keyword_test_utf8_word_begin) { // We make sure that the search works well even if the character is not ASCII MWDialogue::KeywordSearch search; - search.seed("états", 0); - search.seed("ïrradiés", 0); - search.seed("ça nous déçois", 0); + search.seed("états", 0); + search.seed("ïrradiés", 0); + search.seed("ça nous déçois", 0); - std::string text = "les nations unis ont réunis le monde entier, états units inclus pour parler du problème des gens ïrradiés et ça nous déçois"; + std::string text = "les nations unis ont réunis le monde entier, états units inclus pour parler du problème des gens ïrradiés et ça nous déçois"; std::vector::Match> matches; search.highlightKeywords(text.begin(), text.end(), matches); ASSERT_TRUE (matches.size() == 3); - ASSERT_TRUE(std::string( matches[0].mBeg, matches[0].mEnd) == "états"); - ASSERT_TRUE(std::string( matches[1].mBeg, matches[1].mEnd) == "ïrradiés"); - ASSERT_TRUE(std::string( matches[2].mBeg, matches[2].mEnd) == "ça nous déçois"); - - //ASSERT_TRUE (std::string(matches.front().mBeg, matches.front().mEnd) == "bar lock"); + ASSERT_TRUE(std::string( matches[0].mBeg, matches[0].mEnd) == "états"); + ASSERT_TRUE(std::string( matches[1].mBeg, matches[1].mEnd) == "ïrradiés"); + ASSERT_TRUE(std::string( matches[2].mBeg, matches[2].mEnd) == "ça nous déçois"); } From 2d329548887ff91b38a2b757bf216ac1065b20ee Mon Sep 17 00:00:00 2001 From: "florent.teppe" Date: Mon, 20 Sep 2021 19:56:47 +0200 Subject: [PATCH 5/7] Replaced Assert_true with expect_eq --- .../mwdialogue/test_keywordsearch.cpp | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/apps/openmw_test_suite/mwdialogue/test_keywordsearch.cpp b/apps/openmw_test_suite/mwdialogue/test_keywordsearch.cpp index d633feecd9..62b6f67aae 100644 --- a/apps/openmw_test_suite/mwdialogue/test_keywordsearch.cpp +++ b/apps/openmw_test_suite/mwdialogue/test_keywordsearch.cpp @@ -27,9 +27,9 @@ TEST_F(KeywordSearchTest, keyword_test_conflict_resolution) search.highlightKeywords(text.begin(), text.end(), matches); // Should contain: "foo bar", "lock switch" - ASSERT_TRUE (matches.size() == 2); - ASSERT_TRUE (std::string(matches.front().mBeg, matches.front().mEnd) == "foo bar"); - ASSERT_TRUE (std::string(matches.rbegin()->mBeg, matches.rbegin()->mEnd) == "lock switch"); + EXPECT_EQ (matches.size() , 2); + EXPECT_EQ (std::string(matches.front().mBeg, matches.front().mEnd) , "foo bar"); + EXPECT_EQ (std::string(matches.rbegin()->mBeg, matches.rbegin()->mEnd) , "lock switch"); } TEST_F(KeywordSearchTest, keyword_test_conflict_resolution2) @@ -43,8 +43,8 @@ TEST_F(KeywordSearchTest, keyword_test_conflict_resolution2) std::vector::Match> matches; search.highlightKeywords(text.begin(), text.end(), matches); - ASSERT_TRUE (matches.size() == 1); - ASSERT_TRUE (std::string(matches.front().mBeg, matches.front().mEnd) == "dwemer language"); + EXPECT_EQ (matches.size() , 1); + EXPECT_EQ (std::string(matches.front().mBeg, matches.front().mEnd) , "dwemer language"); } @@ -62,8 +62,8 @@ TEST_F(KeywordSearchTest, keyword_test_conflict_resolution3) std::vector::Match> matches; search.highlightKeywords(text.begin(), text.end(), matches); - ASSERT_TRUE (matches.size() == 1); - ASSERT_TRUE (std::string(matches.front().mBeg, matches.front().mEnd) == "bar lock"); + EXPECT_EQ (matches.size() , 1); + EXPECT_EQ (std::string(matches.front().mBeg, matches.front().mEnd) , "bar lock"); } @@ -81,8 +81,8 @@ TEST_F(KeywordSearchTest, keyword_test_utf8_word_begin) std::vector::Match> matches; search.highlightKeywords(text.begin(), text.end(), matches); - ASSERT_TRUE (matches.size() == 3); - ASSERT_TRUE(std::string( matches[0].mBeg, matches[0].mEnd) == "états"); - ASSERT_TRUE(std::string( matches[1].mBeg, matches[1].mEnd) == "ïrradiés"); - ASSERT_TRUE(std::string( matches[2].mBeg, matches[2].mEnd) == "ça nous déçois"); + EXPECT_EQ (matches.size() , 3); + EXPECT_EQ (std::string( matches[0].mBeg, matches[0].mEnd) , "états"); + EXPECT_EQ (std::string( matches[1].mBeg, matches[1].mEnd) , "ïrradiés"); + EXPECT_EQ (std::string( matches[2].mBeg, matches[2].mEnd) , "ça nous déçois"); } From d9c3ba03f4e846128d318bd09d94a8a900d52899 Mon Sep 17 00:00:00 2001 From: "florent.teppe" Date: Mon, 20 Sep 2021 21:01:28 +0200 Subject: [PATCH 6/7] Uses limits instead of climits --- apps/openmw/mwdialogue/keywordsearch.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/openmw/mwdialogue/keywordsearch.hpp b/apps/openmw/mwdialogue/keywordsearch.hpp index fc6f2a91e8..8ba3349bc8 100644 --- a/apps/openmw/mwdialogue/keywordsearch.hpp +++ b/apps/openmw/mwdialogue/keywordsearch.hpp @@ -1,9 +1,9 @@ #ifndef GAME_MWDIALOGUE_KEYWORDSEARCH_H #define GAME_MWDIALOGUE_KEYWORDSEARCH_H -#include -#include #include +#include +#include #include #include #include // std::reverse @@ -71,7 +71,7 @@ public: static bool isWhitespaceUTF8(const int utf8Char) { - if (utf8Char >= 0 && utf8Char <= UCHAR_MAX) + if (utf8Char >= 0 && utf8Char <= static_cast( std::numeric_limits::max())) { //That function has undefined behavior if the character doesn't fit in unsigned char return std::isspace(utf8Char); From 5620e8f0e275db5b6b91d8b3d9bbf3fe8621b1b7 Mon Sep 17 00:00:00 2001 From: "florent.teppe" Date: Sun, 26 Sep 2021 20:40:11 +0200 Subject: [PATCH 7/7] Added the fix to the changelog, and my name to the authors --- AUTHORS.md | 1 + CHANGELOG.md | 1 + 2 files changed, 2 insertions(+) diff --git a/AUTHORS.md b/AUTHORS.md index 62121a797b..a1f00bca10 100644 --- a/AUTHORS.md +++ b/AUTHORS.md @@ -80,6 +80,7 @@ Programmers Federico Guerra (FedeWar) Fil Krynicki (filkry) Finbar Crago (finbar-crago) + Florent Teppe (Tetramir) Florian Weber (Florianjw) Frédéric Chardon (fr3dz10) Gaëtan Dezeiraud (Brouilles) diff --git a/CHANGELOG.md b/CHANGELOG.md index f81a490b49..80fc4aac02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ Bug #6184: Command and Calm and Demoralize and Frenzy and Rally magic effects inconsistencies with vanilla Bug #6197: Infinite Casting Loop Bug #6273: Respawning NPCs rotation is inconsistent + Bug #6289: Keyword search in dialogues expected the text to be all ASCII characters Feature #2554: Modifying an object triggers the instances table to scroll to the corresponding record Feature #2780: A way to see current OpenMW version in the console Feature #3616: Allow Zoom levels on the World Map