Try to add comment filter for search in event list #1398

Bouh · 2020-01-26T19:27:32Z

I feel like I'm not far from the solution.

As I don't know C++ I looked at the other functions and I reproduced what I understood from them.

I don't know how to debug a variable, it would have helped me to know where I am and the state of my values, or at least their structure.

I think I'm in gd::CommentEvent, but according to errors during compilation I'm mostly in the parent class gd::BaseEvent ?

4ian · 2020-01-26T22:32:26Z

That's a good start but there are misconceptions about what an event is :)
The idea is:

You add a method to the gd::BaseEvent class (Event.h) (which is the base class from which all events are deriving) that is there to extract the strings from an event. By default, just return an empty list.
You redefine this method in the CommentEvent to returns the strings that are the comments.
In the class doing the search, you're iterating on events and calling the method to extract the strings for all events (most events will nothing, but comments will return the comments content). for each event, you iterate on this list and do the search in these strings.

4ian · 2020-01-26T22:34:05Z

something like GetEvent() is not a member of baseEvent

This means "You try to call GetEvent() on something, but this something is a gd::BaseEvent, and I can't find any method called GetEvent". So this means that you already have an event :) No need then to call GetEvent(), because you already have one ;)

Bouh · 2020-01-28T17:06:07Z

Core/GDCore/Events/Builtin/CommentEvent.h

-  virtual std::vector<const gd::EventsList*> GetAllCommentsVectors() const;
-  virtual std::vector<gd::EventsList*> GetAllCommentsVectors();
+  virtual std::vector<const gd::String*> GetAllSearchableStrings(BaseEvent &event) const;
+  virtual std::vector<gd::String*> GetAllSearchableStrings( BaseEvent &event);


Not sure here i guess there are mistakes in and after parenthesis.
I guess also one of them should return the vector of string ?
I didn't understand why two similar lines. with Const and without.

As said in another comment:

The one that is const will tell the compiler "please check that the object is not modified". It also tell the compiler "allow me to call this method on a const object". If you do:

class A { void methodNonConst(); void methodThatIsConst() const; }

then you can do:

void someOtherFunction(const A &a) { // Here I say "I take a reference to A, and I won't modify it" a.methodNonConst(); // NOT allowed, compiler will reject it. You're trying to modify the object it seems! a.methodThatIsConst(); // Fine! Nothing will be modified }

Note that if you pass just a reference, there is no difference:

void someOtherFunction(A &a) { // Here I say "I take a reference to A, and I maybe modify it" a.methodNonConst(); // Allowed. The object might be modified a.methodThatIsConst(); // Also allowed! }

This is called "const-correctness": https://www.cprogramming.com/tutorial/const_correctness.html
(don't try to understand everything at first)

You also need only one method here (which can be const as not changing anything):

virtual std::vector<gd::String> GetAllSearchableStrings() const;

Core/GDCore/Events/Event.h

Core/GDCore/Events/Builtin/CommentEvent.cpp

Bouh · 2020-01-28T17:13:39Z

Write c++ code when i didn't read one tutorial about it is not a good idea lol.

Core/GDCore/Events/Builtin/CommentEvent.cpp

Core/GDCore/Events/Builtin/CommentEvent.h

4ian · 2020-01-28T20:19:03Z

Core/GDCore/IDE/Events/EventsRefactorer.cpp

+                                               gd::String search,
+                                               bool matchCase) {
+  // See later for "BuiltinCommonInstructions::Group" when Comment works
+  if (event.GetType() == = "BuiltinCommonInstructions::Comment") {


No need for this: GetAllSearchableStrings is a method that was added in BaseEvent, meaning that all the events now have it!
And it's overriden by CommentEvent, but this method does not care who is overriding it.

If in the future, some events is redefining GetAllSearchableStrings, this will work without any changes.

So you can remove this if (also, no triple equal in C++, just ==)

4ian · 2020-01-28T20:19:51Z

Core/GDCore/IDE/Events/EventsRefactorer.cpp

+                                               bool matchCase) {
+  // See later for "BuiltinCommonInstructions::Group" when Comment works
+  if (event.GetType() == = "BuiltinCommonInstructions::Comment") {
+    for (gd::String str : events[id].GetAllSearchableStrings()) {


Seems like you forgot to iterate on events. events is an gd::EventsList, which, like the name indicates, is a "list", almost an array.

Add the missing for, and this can be just:

for (gd::String str : event.GetAllSearchableStrings()) {

if I suppose you named "event" the "gd::BaseEvent" variable that you will get with "GetEvent(index)" on events

4ian · 2020-01-28T20:21:37Z

Core/GDCore/IDE/Events/EventsRefactorer.h

@@ -202,6 +203,11 @@ class GD_CORE_API EventsRefactorer {
                                       gd::InstructionsList& conditions,
                                       gd::String search,
                                       bool matchCase);
+  static bool GetAllSearchableStrings(gd::ObjectsContainer& project,


You will have to change Bindings.idl to expose this new parameter to JavaScript

4ian · 2020-01-28T20:27:07Z

Write c++ code when i didn't read one tutorial about it is not a good idea lol.

Hopefully these changes are specific enough so that you can do without.. but surely a good idea to get a quick tutorial or two about C++. Will be interesting to see concepts that don't exist in JS and will surely remind you of stuff that you've seen in JS :)
For example, in C++ variables can be passed by value (Car myCar - this copies myCar in memory), reference (Car & car, this does not copy in memory), const-reference (const Car& car, this does not copy in memory and forbid you to change the object) or pointer (Car* car) - though pointer are avoided as much as possible because they can be null.
In JavaScript, all objects and arrays are passed as... reference. But primitive types (boolean, number, strings) are copied (passed by "value").

Bouh · 2020-01-30T18:23:14Z

There are problems
If I understand :

GetAllSearchableStrings() shouldn't be define in EventsRefectorer at all only in BaseEvent & CommentEvent
SearchStringInComments() is now missing in EventsRefectorer O.o

When i get stringsVectors from
vector<gd::String> stringsVectors = events[i].GetAllSearchableStrings();
I need to loop on each string in stringsVectors and pass one string to the missing SearchStringInComments() ?

SearchStringInComments() should be define in EventsRefectorer.cpp & .h ?

I think 'ive mix SearchStringInComments() and GetAllSearchableStrings()

4ian · 2020-01-30T21:36:52Z

GDevelop.js/Bindings/Bindings.idl

@@ -1528,7 +1528,8 @@ interface EventsRefactorer {
    void STATIC_RenameObjectInEvents([Const, Ref] Platform platform, [Ref] ObjectsContainer project, [Ref] ObjectsContainer layout, [Ref] EventsList events, [Const] DOMString oldName, [Const] DOMString newName);
    void STATIC_RemoveObjectInEvents([Const, Ref] Platform platform, [Ref] ObjectsContainer project, [Ref] ObjectsContainer layout, [Ref] EventsList events, [Const] DOMString name);
    void STATIC_ReplaceStringInEvents([Ref] ObjectsContainer project, [Ref] ObjectsContainer layout, [Ref] EventsList events, [Const] DOMString toReplace, [Const] DOMString newString, boolean matchCase, boolean inConditions, boolean inActions);
-    [Value] VectorEventsSearchResult STATIC_SearchInEvents([Ref] ObjectsContainer project, [Ref] ObjectsContainer layout, [Ref] EventsList events, [Const] DOMString search, boolean matchCase, boolean inConditions, boolean inActions);
+    [Value] VectorEventsSearchResult STATIC_SearchInEvents([Ref] ObjectsContainer project, [Ref] ObjectsContainer layout, [Ref] EventsList events, [Const] DOMString search, boolean matchCase, boolean inConditions, boolean inActions, boolean inComments);
+    [Value] VectorEventsSearchResult STATIC_GetAllSearchableStrings([Ref] ObjectsContainer project, [Ref] ObjectsContainer layout, [Ref] EventsList events, [Const] DOMString search, boolean matchCase);


You dont' need this line - and there is no "GetAllSearchableStrings" in EventsRefactorer, why adding it? :)

newIDE/app/src/EventsSheet/SearchPanel.js

4ian · 2020-01-30T21:45:02Z

Core/GDCore/IDE/Events/EventsRefactorer.cpp

+    if (inComments) {
+      vector<gd::String> stringsVectors = events[i].GetAllSearchableStrings();
+
+      for (std::size_t j = 0; j < stringsVectors.size(); ++j) {


Iterating here is doable, but then you iterate also in SearchStringInComments... you have to choose where! :)
Remove this loop

4ian · 2020-01-30T21:45:40Z

Core/GDCore/IDE/Events/EventsRefactorer.cpp

@@ -631,6 +632,21 @@ vector<EventsSearchResult> EventsRefactorer::SearchInEvents(
      }
    }

+    if (inComments) {
+      vector<gd::String> stringsVectors = events[i].GetAllSearchableStrings();


You're already doing this in SearchStringInComments, you can remove that

4ian · 2020-01-30T21:46:42Z

Core/GDCore/IDE/Events/EventsRefactorer.cpp

+
+      for (std::size_t j = 0; j < stringsVectors.size(); ++j) {
+        if (!eventAddedInResults &&
+            SearchStringInComments(


Pass just the event, not a string:

project, layout, events[i], search, matchCase

4ian · 2020-01-30T21:47:24Z

Core/GDCore/IDE/Events/EventsRefactorer.cpp

@@ -711,6 +728,24 @@ bool EventsRefactorer::SearchStringInConditions(
  return false;
 }

+bool EventsRefactorer::SearchStringInComments(gd::ObjectsContainer& project,


Rename this to SearchStringInEvents
There is no notion of "comments" here. The strings might be in the future strings coming from somewhere else... sure for now only comments have strings, but that's a coincidence :)

4ian · 2020-01-30T21:47:46Z

Core/GDCore/IDE/Events/EventsRefactorer.cpp

@@ -711,6 +728,24 @@ bool EventsRefactorer::SearchStringInConditions(
  return false;
 }

+bool EventsRefactorer::SearchStringInComments(gd::ObjectsContainer& project,
+                                               gd::ObjectsContainer& layout,
+                                               gd::EventsList& events,


Pass a gd::BaseEvent& event. You don't need to pass a list of event, just the event where to search in.

4ian · 2020-01-30T21:47:59Z

Core/GDCore/IDE/Events/EventsRefactorer.cpp

+                                               gd::EventsList& events,
+                                               gd::String search,
+                                               bool matchCase) {
+  for (gd::BaseEvent event : events) {


No need to iterate, just pass the event.
You can remove this loop

Core/GDCore/IDE/Events/EventsRefactorer.cpp

4ian · 2020-01-30T21:53:21Z

GetAllSearchableStrings() shouldn't be define in EventsRefectorer at all only in BaseEvent & CommentEvent

Yes

When i get stringsVectors from
vectorgd::String stringsVectors = events[i].GetAllSearchableStrings();
I need to loop on each string in stringsVectors and pass one string to the missing SearchStringInComments() ?

No, this method will take care of iterating. Just pass it the event.

SearchStringInComments() should be define in EventsRefectorer.cpp & .h ?

Yes, everything that is in a cpp file must be in the .h

I think 'ive mix SearchStringInComments() and GetAllSearchableStrings()

Yes 😄Think like this:

GetAllSearchableStrings is something that is in the BaseEvent (the common basis for all events) (and it's redefine by CommentEvent, but we don't need to know that). It has on role, which is "When I call you, you give me all the texts of the event please."
SearchStringInComments is a helper method. It's role: "When I call you, I'll pass you an event and a string called search (plus a few other things that you might not use, like the project). Please tell me (by sending me true or false) if the event contains the search".
- To do this, this method has to ask the event "can you please give me all the texts that you have??"... and good news, we just made a method for that! It's called GetAllSearchableStrings and it's available on all events! So we don't event care what kind of event we're dealing with :)

Bouh · 2020-01-31T22:58:07Z

It's works !
Before merging this one, can i add rename Comments filter by Comments&Groups ?
Or i create a new button for Groups ?
Also Filter by is fine, if you prefer Search by i can replace it ^^

Bouh · 2020-01-31T23:00:48Z

I need just add and adapt GetAllSearchableStrings() in GroupEvent .cpp and .h :)

Bouh · 2020-01-31T23:17:26Z

Uumh a group is also a comment field in the IDE, so i get name from group event, and com from comment event, both on same filter option.

4ian · 2020-01-31T23:53:46Z

Also Filter by is fine, if you prefer Search by i can replace it ^^

"Search in" would be good - we're doing a search in events, and precisely in some part of them :)

I need just add and adapt GetAllSearchableStrings() in GroupEvent .cpp and .h :)

Yes that looks good! :)

Before merging this one, can i add rename Comments filter by Comments&Groups ?

In the UI yes - but we'll run out of place I'm afraid?

In the code, instead of "inComments", name the parameter "inEventStrings" to be explicit that this is not only comments (and not only groups, can be any event with a string).
Also clean the code that is commented and should be good :)

newIDE/app/src/EventsSheet/SearchPanel.js

newIDE/app/src/EventsSheet/EventsSearcher.js

4ian · 2020-02-01T00:40:44Z

Seems like we're almost there 👍

Bouh · 2020-02-01T00:46:53Z

Next option will be search in expressions and search bar will be complete!
In a next PR later.

4ian · 2020-02-01T18:45:42Z

Thanks for working on this 👍👍
Will be very useful for searching in large events sheets.

Bouh · 2020-02-01T19:04:47Z

Thanks for the help! 🐱‍💻

Try to add comment filter for search in event list

2271f8d

Review #1

7e2983e

Bouh commented Jan 28, 2020

View reviewed changes

Core/GDCore/Events/Event.h Show resolved Hide resolved

Core/GDCore/Events/Builtin/CommentEvent.cpp Outdated Show resolved Hide resolved

4ian reviewed Jan 28, 2020

View reviewed changes

Core/GDCore/Events/Builtin/CommentEvent.cpp Outdated Show resolved Hide resolved

4ian reviewed Jan 28, 2020

View reviewed changes

Core/GDCore/Events/Builtin/CommentEvent.cpp Outdated Show resolved Hide resolved

4ian reviewed Jan 28, 2020

View reviewed changes

Core/GDCore/Events/Builtin/CommentEvent.cpp Outdated Show resolved Hide resolved

4ian reviewed Jan 28, 2020

View reviewed changes

Core/GDCore/Events/Builtin/CommentEvent.h Outdated Show resolved Hide resolved

4ian reviewed Jan 28, 2020

View reviewed changes

Review #2

089fa60

SearchStringInComments() is back

da03e19

4ian reviewed Jan 30, 2020

View reviewed changes

newIDE/app/src/EventsSheet/SearchPanel.js Show resolved Hide resolved

4ian reviewed Jan 30, 2020

View reviewed changes

Review 4ian#3

5d2cd25

Add groups

9a508a2

Bouh force-pushed the search_in_comments branch from adb343b to cfd2bb4 Compare February 1, 2020 00:01

Clean code & rename inEventStrings & search in

eb2504b

Bouh force-pushed the search_in_comments branch from cfd2bb4 to eb2504b Compare February 1, 2020 00:03

4ian reviewed Feb 1, 2020

View reviewed changes

newIDE/app/src/EventsSheet/SearchPanel.js Outdated Show resolved Hide resolved

4ian reviewed Feb 1, 2020

View reviewed changes

newIDE/app/src/EventsSheet/EventsSearcher.js Outdated Show resolved Hide resolved

4ian added the 🏁PR almost ready: final fixes The PR is almost ready and needs final fixes and/or polishing before merge label Feb 1, 2020

Consistent searchInEventStrings & "Texts"

9b39d06

4ian added 4 commits February 1, 2020 10:29

Update comments about events

40f57f5

SearchStringInEvents -> SearchStringInEvent (1/2)

63e60ec

SearchStringInEvents -> SearchStringInEvent (2/2)

e8a8b50

SearchStringInEvents -> SearchStringInEvent (3/3)

a71a868

4ian merged commit ce4fdbe into 4ian:master Feb 1, 2020

Bouh deleted the search_in_comments branch February 1, 2020 19:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Try to add comment filter for search in event list #1398

Try to add comment filter for search in event list #1398

Bouh commented Jan 26, 2020

4ian commented Jan 26, 2020

4ian commented Jan 26, 2020

Bouh Jan 28, 2020

4ian Jan 28, 2020

Bouh commented Jan 28, 2020

4ian Jan 28, 2020

4ian Jan 28, 2020

4ian Jan 28, 2020

4ian Jan 28, 2020

4ian commented Jan 28, 2020

Bouh commented Jan 30, 2020 •

edited

4ian Jan 30, 2020

4ian Jan 30, 2020

4ian Jan 30, 2020

4ian Jan 30, 2020

4ian Jan 30, 2020

4ian Jan 30, 2020

4ian Jan 30, 2020

4ian commented Jan 30, 2020

Bouh commented Jan 31, 2020 •

edited

Bouh commented Jan 31, 2020 •

edited

Bouh commented Jan 31, 2020

4ian commented Jan 31, 2020

4ian commented Feb 1, 2020

Bouh commented Feb 1, 2020 •

edited

4ian commented Feb 1, 2020

Bouh commented Feb 1, 2020

Try to add comment filter for search in event list #1398

Try to add comment filter for search in event list #1398

Conversation

Bouh commented Jan 26, 2020

4ian commented Jan 26, 2020

4ian commented Jan 26, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bouh commented Jan 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

4ian commented Jan 28, 2020

Bouh commented Jan 30, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

4ian commented Jan 30, 2020

Bouh commented Jan 31, 2020 • edited

Bouh commented Jan 31, 2020 • edited

Bouh commented Jan 31, 2020

4ian commented Jan 31, 2020

4ian commented Feb 1, 2020

Bouh commented Feb 1, 2020 • edited

4ian commented Feb 1, 2020

Bouh commented Feb 1, 2020

Bouh commented Jan 30, 2020 •

edited

Bouh commented Jan 31, 2020 •

edited

Bouh commented Jan 31, 2020 •

edited

Bouh commented Feb 1, 2020 •

edited